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