diff --git a/git_auth.py b/git_auth.py index fb92bd0771..ad90e9bc56 100644 --- a/git_auth.py +++ b/git_auth.py @@ -340,6 +340,9 @@ class ConfigWizard(object): self._configure_sso(parts, scope=scope) return _ConfigInfo(method=_ConfigMethod.SSO) self._configure_oauth(parts, scope=scope) + if scope == 'global': + # Clear any potential overriding local config + self._clear_local_host_config(parts) return _ConfigInfo(method=_ConfigMethod.OAUTH) def _configure_sso(self, parts: urllib.parse.SplitResult, *, @@ -358,6 +361,20 @@ class ConfigWizard(object): self._set_url_rewrite_override(parts, scope=scope) self._clear_sso_rewrite(parts, scope=scope) + def _clear_local_host_config(self, parts: urllib.parse.SplitResult): + """Clear auth config for one Gerrit host. + + This is used to clear any local config that might override a + correct global config, as we default to configuring the global + config if the local config has the same email, but the local + config might have stale settings. + """ + # Skip this if we're not inside a Git repo. + if not self._remote_url_func(): + return + self._clear_url_rewrite_override(parts, scope='local') + self._clear_sso_rewrite(parts, scope='local') + # Fixing competing auth def _fix_netrc(self) -> None: diff --git a/tests/git_auth_test.py b/tests/git_auth_test.py index 4b6f59ea4d..852a3ab0d9 100755 --- a/tests/git_auth_test.py +++ b/tests/git_auth_test.py @@ -15,6 +15,7 @@ import sys import tempfile from typing import Iterable import unittest +from unittest import mock import urllib.parse sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) @@ -101,7 +102,7 @@ class TestConfigWizard(unittest.TestCase): } self.assertEqual(self.global_state, want) - def test_configure_review_sso_global(self): + def test_configure_sso_global_with_review_host(self): parts = urllib.parse.urlsplit( 'https://chromium-review.googlesource.com/chromium/tools/depot_tools.git' ) @@ -124,7 +125,7 @@ class TestConfigWizard(unittest.TestCase): } self.assertEqual(self.global_state, want) - def test_configure_review_oauth_global(self): + def test_configure_oauth_global_with_review_host(self): parts = urllib.parse.urlsplit( 'https://chromium-review.googlesource.com/chromium/tools/depot_tools.git' ) @@ -190,6 +191,68 @@ class TestConfigWizard(unittest.TestCase): 'https://chromium-review.googlesource.com/chromium/tools/depot_tools.git', ]) + @mock.patch('git_auth.ConfigWizard._check_use_sso', spec=True) + def test_configure_host_clears_conflicting_local_stale(self, check): + parts = urllib.parse.urlsplit( + 'https://chromium.googlesource.com/chromium/tools/depot_tools.git') + email = 'columbina@example.com' + + check.return_value = True + self.wizard._configure_host(parts, email, scope='local') + # Check local rule is created + self.assertEqual( + scm.GIT.GetConfigList(os.getcwd(), 'url.sso://chromium/.insteadof'), + [ + 'https://chromium.googlesource.com/', + 'https://chromium-review.googlesource.com/', + ]) + + check.return_value = False + self.wizard._configure_host(parts, email, scope='global') + want = { + 'credential.https://chromium.googlesource.com.helper': ['', 'luci'], + 'credential.https://chromium.googlesource.com.usehttppath': ['yes'], + } + self.assertEqual(self.global_state, want) + # Ensure the local rule gets purged + self.assertEqual( + scm.GIT.GetConfigList(os.getcwd(), 'url.sso://chromium/.insteadof'), + []) + + @mock.patch('git_auth.ConfigWizard._check_use_sso', spec=True) + def test_configure_host_outside_repo_keeps_local_stale(self, check): + parts = urllib.parse.urlsplit( + 'https://chromium.googlesource.com/chromium/tools/depot_tools.git') + email = 'columbina@example.com' + + check.return_value = True + self.wizard._configure_host(parts, email, scope='local') + # Check local rule is created + self.assertEqual( + scm.GIT.GetConfigList(os.getcwd(), 'url.sso://chromium/.insteadof'), + [ + 'https://chromium.googlesource.com/', + 'https://chromium-review.googlesource.com/', + ]) + + check.return_value = False + wizard = git_auth.ConfigWizard(ui=self.ui, remote_url_func=lambda: '') + wizard._configure_host(parts, email, scope='global') + want = { + 'credential.https://chromium.googlesource.com.helper': ['', 'luci'], + 'credential.https://chromium.googlesource.com.usehttppath': ['yes'], + } + self.assertEqual(self.global_state, want) + # Check the local rule remains. + # This test is wonky because the scm.GIT mock always assumes we're in a repo + self.assertEqual( + scm.GIT.GetConfigList(os.getcwd(), 'url.sso://chromium/.insteadof'), + [ + 'https://chromium.googlesource.com/', + 'https://chromium-review.googlesource.com/', + ]) + + def test_check_gitcookies_same(self): with tempfile.NamedTemporaryFile() as gitcookies: self.wizard._gitcookies = lambda: gitcookies.name