From 91937bf19634f1ae36592fb1f01ae50f85134494 Mon Sep 17 00:00:00 2001 From: Allen Li Date: Sat, 20 Jul 2024 00:48:36 +0000 Subject: [PATCH] [git_cl] Parametrize email in ShouldUseSSO Moves the dependency on Git+cwd up the call stack Bug: b/351071334 Change-Id: Ia313f9d4720ee10398b757217c333118e9fc7341 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5723091 Reviewed-by: Yiwei Zhang Reviewed-by: Robbie Iannucci Commit-Queue: Allen Li --- gerrit_util.py | 9 ++++----- git_cl.py | 3 ++- tests/gerrit_util_test.py | 28 ++++++++++++++-------------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/gerrit_util.py b/gerrit_util.py index 5f59e2ce4..c09c331b6 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -171,8 +171,8 @@ class SSOHelper(object): ssoHelper = SSOHelper() -def ShouldUseSSO(cwd: str, host: str) -> bool: - """Return True if we should use SSO for the current user.""" +def ShouldUseSSO(host: str, email: str) -> bool: + """Return True if we should use SSO for the given Gerrit host and user.""" LOGGER.debug("Determining whether we should use SSO...") if not newauth.Enabled(): LOGGER.debug("SSO=False: not opted in") @@ -183,7 +183,6 @@ def ShouldUseSSO(cwd: str, host: str) -> bool: if not ssoHelper.find_cmd(): LOGGER.debug("SSO=False: no SSO command") return False - email = scm.GIT.GetConfig(cwd, 'user.email', default='') if email.endswith('@google.com'): LOGGER.debug("SSO=True: email is google.com") return True @@ -363,9 +362,9 @@ class SSOAuthenticator(_Authenticator): def is_applicable(cls, *, conn: Optional[HttpConn] = None) -> bool: if not cls._resolve_sso_cmd(): return False + email: str = scm.GIT.GetConfig(os.getcwd(), 'user.email', default='') if conn is not None: - return ShouldUseSSO(os.getcwd(), conn.host) - email = scm.GIT.GetConfig(os.getcwd(), 'user.email', default='') + return ShouldUseSSO(conn.host, email) return email.endswith('@google.com') @classmethod diff --git a/git_cl.py b/git_cl.py index 23d2294cc..544cfd1ea 100755 --- a/git_cl.py +++ b/git_cl.py @@ -3759,7 +3759,8 @@ class GitAuthConfigChanger(object): """Infer default mode to use.""" if not newauth.Enabled(): return GitConfigMode.OLD_AUTH - if gerrit_util.ShouldUseSSO(cwd, gerrit_host): + email: str = scm.GIT.GetConfig(cwd, 'user.email', default='') + if gerrit_util.ShouldUseSSO(gerrit_host, email): return GitConfigMode.NEW_AUTH_SSO return GitConfigMode.NEW_AUTH diff --git a/tests/gerrit_util_test.py b/tests/gerrit_util_test.py index 4385d51e8..a8fcb2af4 100755 --- a/tests/gerrit_util_test.py +++ b/tests/gerrit_util_test.py @@ -736,36 +736,36 @@ class ShouldUseSSOTest(unittest.TestCase): def testDisabled(self): self.newauth.return_value = False - self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host')) + self.assertFalse(gerrit_util.ShouldUseSSO('fake-host', '')) def testMissingCommand(self): self.sso.return_value = 'fake-host' - self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host')) + self.assertFalse(gerrit_util.ShouldUseSSO('fake-host', '')) - @mock.patch('scm.GIT.GetConfig', return_value='firefly@google.com') - def testGoogle(self, _): - self.assertTrue(gerrit_util.ShouldUseSSO('/', 'fake-host')) + def testGoogle(self): + self.assertTrue( + gerrit_util.ShouldUseSSO('fake-host', 'firefly@google.com')) - @mock.patch('scm.GIT.GetConfig', return_value='firefly@gmail.com') - def testGmail(self, _): - self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host')) + def testGmail(self): + self.assertFalse( + gerrit_util.ShouldUseSSO('fake-host', 'firefly@gmail.com')) @mock.patch('gerrit_util.GetAccountEmails', return_value=[{ 'email': 'firefly@chromium.org' }]) - @mock.patch('scm.GIT.GetConfig', return_value='firefly@chromium.org') - def testLinkedChromium(self, _cfg, email): - self.assertTrue(gerrit_util.ShouldUseSSO('/', 'fake-host')) + def testLinkedChromium(self, email): + self.assertTrue( + gerrit_util.ShouldUseSSO('fake-host', 'firefly@chromium.org')) email.assert_called_with('fake-host', 'self', authenticator=mock.ANY) @mock.patch('gerrit_util.GetAccountEmails', return_value=[{ 'email': 'firefly@google.com' }]) - @mock.patch('scm.GIT.GetConfig', return_value='firefly@chromium.org') - def testUnlinkedChromium(self, _cfg, email): - self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host')) + def testUnlinkedChromium(self, email): + self.assertFalse( + gerrit_util.ShouldUseSSO('fake-host', 'firefly@chromium.org')) email.assert_called_with('fake-host', 'self', authenticator=mock.ANY)