git_auth: Handle conflicting stale local config (reland)

As we prefer configuring the global config, it's possible to leave
stale local config behind if the user is swapping between emails.

Originally:
https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/7213753

Fixed to skip clearing local config when not inside a repo (as that errors).

Bug: 466343784
Change-Id: I8b34d24ba3966c66aeebaf11bd0368dfb4b0da28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/7265721
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Allen Li <ayatane@chromium.org>

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:

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