[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 <yiwzhang@google.com>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Allen Li <ayatane@chromium.org>
changes/91/5723091/5
Allen Li 1 year ago committed by LUCI CQ
parent e7cc4c93c0
commit 91937bf196

@ -171,8 +171,8 @@ class SSOHelper(object):
ssoHelper = SSOHelper() ssoHelper = SSOHelper()
def ShouldUseSSO(cwd: str, host: str) -> bool: def ShouldUseSSO(host: str, email: str) -> bool:
"""Return True if we should use SSO for the current user.""" """Return True if we should use SSO for the given Gerrit host and 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():
LOGGER.debug("SSO=False: not opted in") LOGGER.debug("SSO=False: not opted in")
@ -183,7 +183,6 @@ def ShouldUseSSO(cwd: str, 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
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")
return True return True
@ -363,9 +362,9 @@ class SSOAuthenticator(_Authenticator):
def is_applicable(cls, *, conn: Optional[HttpConn] = None) -> bool: def is_applicable(cls, *, conn: Optional[HttpConn] = None) -> bool:
if not cls._resolve_sso_cmd(): if not cls._resolve_sso_cmd():
return False return False
email: str = scm.GIT.GetConfig(os.getcwd(), 'user.email', default='')
if conn is not None: if conn is not None:
return ShouldUseSSO(os.getcwd(), conn.host) return ShouldUseSSO(conn.host, email)
email = scm.GIT.GetConfig(os.getcwd(), 'user.email', default='')
return email.endswith('@google.com') return email.endswith('@google.com')
@classmethod @classmethod

@ -3759,7 +3759,8 @@ class GitAuthConfigChanger(object):
"""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(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_SSO
return GitConfigMode.NEW_AUTH return GitConfigMode.NEW_AUTH

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

Loading…
Cancel
Save