From 58cb6562e3285d0ded862df46f33006ef74f4a3f Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Wed, 12 Jun 2024 06:50:10 +0000 Subject: [PATCH] Revert "[gerrit_util] Add dogfoodable luci-auth Authenticator." This reverts commit f871d80a7ef17687e688bf8b2adfa7d565287ded. Reason for revert: This is to revert https://crrev.com/c/5624014. Original change's description: > [gerrit_util] Add dogfoodable luci-auth Authenticator. > > Inspired by https://chromium-review.googlesource.com/c/5577744. > > This implementation allows toggling the entire new-auth-stack with > the git configuration: > > [depot-tools] > useNewAuthStack = 1 > > Additionally, you can set: > > [depot-tools] > newAuthSkipSSO = 1 > > To intentionally skip SSOAuthenticator for now while doing local > evaluation of these auth methods. > > This CL was uploaded without gitcookies using the new luci-auth > Authenticator. > > Subsequent CLs will adjust creds-check and EnsureAuthenticated to > work correctly with the new auth stack. > > R=ayatane@google.com > > Bug: 336351842, 336652327 > Change-Id: I0eb6d82ca106ddd114b74f63d8cda4c5a7b70c86 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5590324 > Reviewed-by: Yiwei Zhang > Reviewed-by: Scott Lee > Commit-Queue: Allen Li Bug: 336351842, 336652327 Change-Id: I7c947760a096f48bdac3d640f71e40ad10fe6f3e No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5624015 Owners-Override: Patti Lor Auto-Submit: Takuto Ikuta Commit-Queue: Patti Lor Reviewed-by: Patti Lor Bot-Commit: Rubber Stamper --- auth.py | 4 ++-- gerrit_util.py | 65 ++++++++++++++++---------------------------------- 2 files changed, 23 insertions(+), 46 deletions(-) diff --git a/auth.py b/auth.py index b879a746d..de9ecfd41 100644 --- a/auth.py +++ b/auth.py @@ -107,7 +107,7 @@ class Authenticator(object): return self._access_token # Nope, still expired. Needs user interaction. - logging.debug('Failed to create access token') + logging.error('Failed to create access token') raise LoginRequiredError(self._scopes) def get_id_token(self): @@ -127,7 +127,7 @@ class Authenticator(object): return self._id_token # Nope, still expired. Needs user interaction. - logging.debug('Failed to create id token') + logging.error('Failed to create id token') raise LoginRequiredError() def authorize(self, http, use_id_token=False): diff --git a/gerrit_util.py b/gerrit_util.py index 52cfcb0a6..4b104e66c 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -177,42 +177,24 @@ class Authenticator(object): Probes the local system and its environment and identifies the Authenticator instance to use. """ - use_new_auth = scm.GIT.GetConfig(os.getcwd(), - 'depot-tools.usenewauthstack') == '1' - - # Allow skipping SSOAuthenticator for local testing purposes. - skip_sso = scm.GIT.GetConfig(os.getcwd(), - 'depot-tools.newauthskipsso') == '1' - - authenticators: List[Type[Authenticator]] - - if use_new_auth: - LOGGER.debug('Authenticator.get: using new auth stack.') - authenticators = [ - SSOAuthenticator, - LuciContextAuthenticator, - GceAuthenticator, - LuciAuthAuthenticator, - ] - if skip_sso: - LOGGER.debug('Authenticator.get: skipping SSOAuthenticator.') - authenticators = authenticators[1:] - else: - authenticators = [ - LuciContextAuthenticator, - GceAuthenticator, - CookiesAuthenticator, - ] - + authenticators: List[Type[Authenticator]] = [ + SSOAuthenticator, + + # LUCI Context takes priority since it's normally present only on bots, + # which then must use it. + LuciContextAuthenticator, + + # TODO(crbug.com/1059384): Automatically detect when running on + # cloudtop, and use CookiesAuthenticator instead. + GceAuthenticator, + CookiesAuthenticator, + ] for candidate in authenticators: if candidate.is_applicable(): - LOGGER.debug('Authenticator.get: Selected %s.', - candidate.__name__) return candidate() - auth_names = ', '.join(a.__name__ for a in authenticators) raise ValueError( - f"Could not find suitable authenticator, tried: [{auth_names}].") + f"Could not find suitable authenticator, tried: {authenticators}") class SSOAuthenticator(Authenticator): @@ -252,7 +234,14 @@ class SSOAuthenticator(Authenticator): def is_applicable(cls) -> bool: """If the git-remote-sso binary is in $PATH, we consider this authenticator to be applicable.""" - return bool(cls._resolve_sso_binary_path()) + if scm.GIT.GetConfig(os.getcwd(), 'depot-tools.usenewauthstack') != '1': + LOGGER.debug('SSOAuthenticator: skipping due to missing opt-in.') + return False + + pth = cls._resolve_sso_binary_path() + if pth: + LOGGER.debug('SSOAuthenticator: enabled %r.', pth) + return bool(pth) @classmethod def _parse_config(cls, config: str) -> SSOInfo: @@ -647,18 +636,6 @@ class LuciContextAuthenticator(Authenticator): return '' -class LuciAuthAuthenticator(LuciContextAuthenticator): - """Authenticator implementation that uses `luci-auth` credentials. - - This is the same as LuciContextAuthenticator, except that it is for local - non-google.com developer credentials. - """ - - @staticmethod - def is_applicable(): - return True - - class ReqParams(TypedDict): uri: str method: str