From 522f5a4bd1aa8e74db0f54edc2916776d8ca49df Mon Sep 17 00:00:00 2001 From: Allen Li Date: Fri, 6 Dec 2024 01:39:13 +0000 Subject: [PATCH] gerrit_util: Use git-credential-luci instead of luci-auth git-credential-luci is very similar to luci-auth, except that it is expressly for git/Gerrit. Therefore, it hard codes the scopes needed for git/Gerrit. It's also a separate binary, which makes it more convenient for us to configure it for ReAuth later. Bug: b/382341041 Change-Id: I7de56d3922adac7eb4671849eb6e30be310d4de7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6073043 Reviewed-by: Josip Sokcevic Commit-Queue: Allen Li --- auth.py | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++ gerrit_util.py | 21 ++++++++++++----- git_auth.py | 2 +- git_cl.py | 8 +++---- 4 files changed, 82 insertions(+), 10 deletions(-) diff --git a/auth.py b/auth.py index 7af4998f0..f7bea06f0 100644 --- a/auth.py +++ b/auth.py @@ -3,6 +3,8 @@ # found in the LICENSE file. """Google OAuth2 related functions.""" +from __future__ import annotations + import collections import datetime import functools @@ -10,6 +12,7 @@ import httplib2 import json import logging import os +from typing import Optional import subprocess2 @@ -58,6 +61,22 @@ class LoginRequiredError(Exception): return 'luci-auth login -scopes "%s"' % self.scopes +class GitLoginRequiredError(Exception): + """Interaction with the user is required to authenticate. + + This is for git-credential-luci, not luci-auth. + """ + + def __init__(self): + msg = ('You are not logged in. Please login first by running:\n' + ' %s' % self.login_command) + super(GitLoginRequiredError, self).__init__(msg) + + @property + def login_command(self) -> str: + return 'git-credential-luci login' + + def has_luci_context_local_auth(): """Returns whether LUCI_CONTEXT should be used for ambient authentication.""" ctx_path = os.environ.get('LUCI_CONTEXT') @@ -201,3 +220,45 @@ class Authenticator(object): # stdout/stderr. logging.error('luci-auth token failed: %s', e) return None + + +class GerritAuthenticator(object): + """Object that knows how to refresh access tokens for Gerrit. + + Unlike Authenticator, this is specifically for authenticating Gerrit + requests. + """ + + def __init__(self): + self._access_token: Optional[str] = None + + def get_access_token(self) -> str: + """Returns AccessToken, refreshing it if necessary. + + Raises: + GitLoginRequiredError if user interaction is required. + """ + access_token = self._get_luci_auth_token() + if access_token: + return access_token + logging.debug('Failed to create access token') + raise GitLoginRequiredError() + + def _get_luci_auth_token(self, use_id_token=False) -> Optional[str]: + logging.debug('Running git-credential-luci') + try: + out, err = subprocess2.check_call_out( + ['git-credential-luci', 'get'], + stdout=subprocess2.PIPE, + stderr=subprocess2.PIPE) + logging.debug('git-credential-luci stderr:\n%s', err) + for line in out.decode().splitlines(): + if line.startswith('password='): + return line[len('password='):].rstrip() + logging.error('git-credential-luci did not return a token') + return None + except subprocess2.CalledProcessError as e: + # subprocess2.CalledProcessError.__str__ nicely formats + # stdout/stderr. + logging.error('git-credential-luci failed: %s', e) + return None diff --git a/gerrit_util.py b/gerrit_util.py index 8b459da60..85654050a 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -286,7 +286,7 @@ class _Authenticator(object): SSOAuthenticator(), # GCE detection can't distinguish cloud workstations. GceAuthenticator(), - LuciAuthAuthenticator(), + GitCredsAuthenticator(), NoAuthenticator(), ] if skip_sso: @@ -850,13 +850,24 @@ class LuciContextAuthenticator(_Authenticator): return '' -class LuciAuthAuthenticator(LuciContextAuthenticator): - """_Authenticator implementation that uses `luci-auth` credentials. +class GitCredsAuthenticator(_Authenticator): + """_Authenticator implementation that uses `git-credential-luci` with OAuth. - This is the same as LuciContextAuthenticator, except that it is for local - non-google.com developer credentials. + This is similar to LuciContextAuthenticator, except that it is for + local non-google.com developer credentials. """ + def __init__(self): + self._authenticator = auth.GerritAuthenticator() + + def authenticate(self, conn: HttpConn): + conn.req_headers[ + 'Authorization'] = f'Bearer {self._authenticator.get_access_token()}' + + def debug_summary_state(self) -> str: + # TODO(b/343230702) - report ambient account name. + return '' + @classmethod def gerrit_account_exists(cls, host: str) -> bool: """Return True if the Gerrit account exists. diff --git a/git_auth.py b/git_auth.py index b61265b90..6cb5a8e39 100644 --- a/git_auth.py +++ b/git_auth.py @@ -130,7 +130,7 @@ class ConfigChanger(object): email: str = scm.GIT.GetConfig(cwd, 'user.email') or '' if gerrit_util.ShouldUseSSO(gerrit_host, email): return ConfigMode.NEW_AUTH_SSO - if not gerrit_util.LuciAuthAuthenticator.gerrit_account_exists( + if not gerrit_util.GitCredsAuthenticator.gerrit_account_exists( gerrit_host): return ConfigMode.NO_AUTH return ConfigMode.NEW_AUTH diff --git a/git_cl.py b/git_cl.py index 12f769af4..1075bef44 100755 --- a/git_cl.py +++ b/git_cl.py @@ -3897,12 +3897,12 @@ def CMDcreds_check(parser, args): if gerrit_util.ShouldUseSSO(host, email): print('Detected that we should be using SSO.') else: - print('Detected that we should be using luci-auth.') - a = gerrit_util.LuciAuthAuthenticator() + print('Detected that we should be using git-credential-luci.') + a = gerrit_util.GitCredsAuthenticator() try: a.luci_auth.get_access_token() - except auth.LoginRequiredError as e: - print('NOTE: You are not logged in with luci-auth.') + except auth.GitLoginRequiredError as e: + print('NOTE: You are not logged in with git-credential-luci.') print( 'You will not be able to perform many actions without logging in.' )