From 4b9413003436466d374a306d879338680c54b467 Mon Sep 17 00:00:00 2001 From: Allen Li Date: Wed, 18 Sep 2024 21:57:33 +0000 Subject: [PATCH] [gerrit_util] Add fallback if missing Gerrit account Bug: b/366261039 Change-Id: I7d22c4f03ad9bd837190dee7a511af3437a30434 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5869050 Reviewed-by: Josip Sokcevic Commit-Queue: Allen Li --- auth.py | 7 +++++- gerrit_util.py | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++ git_auth.py | 5 ++++- git_cl.py | 17 +++++++++++++- 4 files changed, 86 insertions(+), 3 deletions(-) diff --git a/auth.py b/auth.py index 8ff207ebe7..7af4998f0d 100644 --- a/auth.py +++ b/auth.py @@ -48,10 +48,15 @@ class Token(collections.namedtuple('Token', [ class LoginRequiredError(Exception): """Interaction with the user is required to authenticate.""" def __init__(self, scopes=OAUTH_SCOPE_EMAIL): + self.scopes = scopes msg = ('You are not logged in. Please login first by running:\n' - ' luci-auth login -scopes "%s"' % scopes) + ' %s' % self.login_command) super(LoginRequiredError, self).__init__(msg) + @property + def login_command(self) -> str: + return 'luci-auth login -scopes "%s"' % self.scopes + def has_luci_context_local_auth(): """Returns whether LUCI_CONTEXT should be used for ambient authentication.""" diff --git a/gerrit_util.py b/gerrit_util.py index 148cddf9e7..8669b8cd03 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -287,6 +287,7 @@ class _Authenticator(object): # GCE detection can't distinguish cloud workstations. GceAuthenticator(), LuciAuthAuthenticator(), + NoAuthenticator(), ] if skip_sso: LOGGER.debug( @@ -812,6 +813,10 @@ class LuciContextAuthenticator(_Authenticator): self._authenticator = auth.Authenticator(' '.join( [auth.OAUTH_SCOPE_EMAIL, auth.OAUTH_SCOPE_GERRIT])) + @property + def luci_auth(self) -> auth.Authenticator: + return self._authenticator + def authenticate(self, conn: HttpConn): conn.req_headers[ 'Authorization'] = f'Bearer {self._authenticator.get_access_token().token}' @@ -828,10 +833,65 @@ class LuciAuthAuthenticator(LuciContextAuthenticator): non-google.com developer credentials. """ + @classmethod + def gerrit_account_exists(cls, host: str) -> bool: + """Return True if the Gerrit account exists. + + This checks the user currently logged in with luci-auth. + If the user is not logged in with luci-auth, returns False. + + This method caches positive results in the user's Git config. + """ + cwd = os.getcwd() + LOGGER.debug("Checking Gerrit account existence for %r", host) + hosts = scm.GIT.GetConfigList(cwd, 'depot-tools.hosthasaccount') + if host in hosts: + # If a user deletes their Gerrit account, then this cache + # might be stale. This should be a rare case and a user can + # just delete this from their Git config. + LOGGER.debug("Using cached account existence for Gerrit host %r", + host) + return True + try: + info = GetAccountDetails(host, authenticator=cls()) + except auth.LoginRequiredError: + LOGGER.debug( + "Cannot check Gerrit account existence; missing luci-auth login" + ) + return False + except GerritError as e: + if e.http_status == 400: + # This is likely because the user doesn't have an + # account on the Gerrit host. + LOGGER.debug( + "Gerrit account check returned 400; likely account missing") + return False + raise + if 'email' not in info: + LOGGER.debug("Gerrit account does not exist on %r", host) + return False + LOGGER.debug("Gerrit account exists on %r", host) + scm.GIT.SetConfig(cwd, 'depot-tools.hostHasAccount', host, append=True) + return True + + def is_applicable(self, *, conn: Optional[HttpConn] = None): + return self.gerrit_account_exists(conn.host) + + +class NoAuthenticator(_Authenticator): + """_Authenticator implementation that does no auth. + """ + @staticmethod def is_applicable(*, conn: Optional[HttpConn] = None): return True + def authenticate(self, conn: HttpConn): + pass + + def debug_summary_state(self) -> str: + return '' + class ChainedAuthenticator(_Authenticator): """_Authenticator that delegates to others in sequence. diff --git a/git_auth.py b/git_auth.py index 885c5d7438..b61265b90a 100644 --- a/git_auth.py +++ b/git_auth.py @@ -36,7 +36,7 @@ class ConfigChanger(object): # # Increment this when making changes to the config, so that reliant # code can determine whether the config needs to be re-applied. - VERSION: int = 2 + VERSION: int = 3 def __init__( self, @@ -130,6 +130,9 @@ 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( + gerrit_host): + return ConfigMode.NO_AUTH return ConfigMode.NEW_AUTH def apply(self, cwd: str) -> None: diff --git a/git_cl.py b/git_cl.py index 771e71f50f..901c12e3a5 100755 --- a/git_cl.py +++ b/git_cl.py @@ -3881,7 +3881,22 @@ def CMDcreds_check(parser, args): _, _ = parser.parse_args(args) if newauth.Enabled(): - git_auth.Configure(os.getcwd(), Changelist()) + cl = Changelist() + git_auth.Configure(os.getcwd(), cl) + # Perform some advisory checks + email = scm.GIT.GetConfig(os.getcwd(), 'user.email') or '' + if not gerrit_util.ShouldUseSSO(cl.GetGerritHost(), email): + a = gerrit_util.LuciAuthAuthenticator() + try: + a.luci_auth.get_access_token() + except auth.LoginRequiredError as e: + print('NOTE: You are not logged in with luci-auth.') + print( + 'You may not be able to perform some actions without logging in.' + ) + print('If you wish to log in, run:') + print(' ' + e.login_command) + print('and re-run this command.') return 0 if newauth.ExplicitlyDisabled(): git_auth.ClearRepoConfig(os.getcwd(), Changelist())