[git_cl] Parametrize cwd in ShouldUseSSO

Remove implicit/global assumptions which could affect tests.

Bug: b/351071334
Change-Id: Ie266228f404b768cb539fdc17dddbbb13693e939
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5723208
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Allen Li <ayatane@chromium.org>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
changes/08/5723208/5
Allen Li 1 year ago committed by LUCI CQ
parent dbebea0aa2
commit 4986d4a74b

@ -171,7 +171,7 @@ class SSOHelper(object):
ssoHelper = SSOHelper() ssoHelper = SSOHelper()
def ShouldUseSSO(host: str) -> bool: def ShouldUseSSO(cwd: str, host: str) -> bool:
"""Return True if we should use SSO for the current user.""" """Return True if we should use SSO for the current user."""
LOGGER.debug("Determining whether we should use SSO...") LOGGER.debug("Determining whether we should use SSO...")
if not newauth.Enabled(): if not newauth.Enabled():
@ -183,7 +183,6 @@ def ShouldUseSSO(host: str) -> bool:
if not ssoHelper.find_cmd(): if not ssoHelper.find_cmd():
LOGGER.debug("SSO=False: no SSO command") LOGGER.debug("SSO=False: no SSO command")
return False return False
cwd = os.getcwd()
email = scm.GIT.GetConfig(cwd, 'user.email', default='') email = scm.GIT.GetConfig(cwd, 'user.email', default='')
if email.endswith('@google.com'): if email.endswith('@google.com'):
LOGGER.debug("SSO=True: email is google.com") LOGGER.debug("SSO=True: email is google.com")
@ -365,7 +364,7 @@ class SSOAuthenticator(_Authenticator):
if not cls._resolve_sso_cmd(): if not cls._resolve_sso_cmd():
return False return False
if conn is not None: if conn is not None:
return ShouldUseSSO(conn.host) return ShouldUseSSO(os.getcwd(), conn.host)
email = scm.GIT.GetConfig(os.getcwd(), 'user.email', default='') email = scm.GIT.GetConfig(os.getcwd(), 'user.email', default='')
return email.endswith('@google.com') return email.endswith('@google.com')

@ -3748,16 +3748,16 @@ class GitAuthConfigChanger(object):
remote_url: str = cl.GetRemoteUrl() remote_url: str = cl.GetRemoteUrl()
return cls( return cls(
mode=cls._infer_mode(gerrit_host), mode=cls._infer_mode(os.getcwd(), gerrit_host),
remote_url=remote_url, remote_url=remote_url,
) )
@staticmethod @staticmethod
def _infer_mode(gerrit_host: str) -> GitConfigMode: def _infer_mode(cwd: str, gerrit_host: str) -> GitConfigMode:
"""Infer default mode to use.""" """Infer default mode to use."""
if not newauth.Enabled(): if not newauth.Enabled():
return GitConfigMode.OLD_AUTH return GitConfigMode.OLD_AUTH
if gerrit_util.ShouldUseSSO(gerrit_host): if gerrit_util.ShouldUseSSO(cwd, gerrit_host):
return GitConfigMode.NEW_AUTH_SSO return GitConfigMode.NEW_AUTH_SSO
return GitConfigMode.NEW_AUTH return GitConfigMode.NEW_AUTH

@ -736,19 +736,19 @@ class ShouldUseSSOTest(unittest.TestCase):
def testDisabled(self): def testDisabled(self):
self.newauth.return_value = False self.newauth.return_value = False
self.assertFalse(gerrit_util.ShouldUseSSO('fake-host')) self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host'))
def testMissingCommand(self): def testMissingCommand(self):
self.sso.return_value = 'fake-host' 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') @mock.patch('scm.GIT.GetConfig', return_value='firefly@google.com')
def testGoogle(self, _): def testGoogle(self, _):
self.assertTrue(gerrit_util.ShouldUseSSO('fake-host')) self.assertTrue(gerrit_util.ShouldUseSSO('/', 'fake-host'))
@mock.patch('scm.GIT.GetConfig', return_value='firefly@gmail.com') @mock.patch('scm.GIT.GetConfig', return_value='firefly@gmail.com')
def testGmail(self, _): def testGmail(self, _):
self.assertFalse(gerrit_util.ShouldUseSSO('fake-host')) self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host'))
@mock.patch('gerrit_util.GetAccountEmails', @mock.patch('gerrit_util.GetAccountEmails',
return_value=[{ return_value=[{
@ -756,7 +756,7 @@ class ShouldUseSSOTest(unittest.TestCase):
}]) }])
@mock.patch('scm.GIT.GetConfig', return_value='firefly@chromium.org') @mock.patch('scm.GIT.GetConfig', return_value='firefly@chromium.org')
def testLinkedChromium(self, _cfg, email): def testLinkedChromium(self, _cfg, email):
self.assertTrue(gerrit_util.ShouldUseSSO('fake-host')) self.assertTrue(gerrit_util.ShouldUseSSO('/', 'fake-host'))
email.assert_called_with('fake-host', 'self', authenticator=mock.ANY) email.assert_called_with('fake-host', 'self', authenticator=mock.ANY)
@mock.patch('gerrit_util.GetAccountEmails', @mock.patch('gerrit_util.GetAccountEmails',
@ -765,7 +765,7 @@ class ShouldUseSSOTest(unittest.TestCase):
}]) }])
@mock.patch('scm.GIT.GetConfig', return_value='firefly@chromium.org') @mock.patch('scm.GIT.GetConfig', return_value='firefly@chromium.org')
def testUnlinkedChromium(self, _cfg, email): def testUnlinkedChromium(self, _cfg, email):
self.assertFalse(gerrit_util.ShouldUseSSO('fake-host')) self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host'))
email.assert_called_with('fake-host', 'self', authenticator=mock.ANY) email.assert_called_with('fake-host', 'self', authenticator=mock.ANY)

Loading…
Cancel
Save