[gerrit_util] Don't try to use SSO on non-googlesource hosts

In case you're getting funny thoughts, these requests would not attach
any cookies anyway because the domain doesn't match.  It just allows
requests to properly fallback to OAuth path.

Bug: b/362741558
Change-Id: Iaf83ad640501ff45671dbc358e676cbeaf04d686
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5824222
Commit-Queue: Allen Li <ayatane@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
changes/22/5824222/4
Allen Li 1 year ago committed by LUCI CQ
parent b89b440351
commit dc8d502d13

@ -179,6 +179,9 @@ def ShouldUseSSO(host: str, email: str) -> bool:
if not newauth.Enabled(): if not newauth.Enabled():
LOGGER.debug("SSO=False: not opted in") LOGGER.debug("SSO=False: not opted in")
return False return False
if not host.endswith('.googlesource.com'):
LOGGER.debug("SSO=False: non-googlesource host %r", host)
return False
if newauth.SkipSSO(): if newauth.SkipSSO():
LOGGER.debug("SSO=False: set skip SSO config") LOGGER.debug("SSO=False: set skip SSO config")
return False return False

@ -752,23 +752,33 @@ class ShouldUseSSOTest(unittest.TestCase):
@mock.patch('newauth.Enabled', return_value=False) @mock.patch('newauth.Enabled', return_value=False)
def testDisabled(self, _): def testDisabled(self, _):
self.assertFalse( self.assertFalse(
gerrit_util.ShouldUseSSO('fake-host', 'firefly@google.com')) gerrit_util.ShouldUseSSO('fake-host.googlesource.com',
'firefly@google.com'))
@mock.patch('gerrit_util.ssoHelper.find_cmd', return_value='') @mock.patch('gerrit_util.ssoHelper.find_cmd', return_value='')
def testMissingCommand(self, _): def testMissingCommand(self, _):
self.assertFalse( self.assertFalse(
gerrit_util.ShouldUseSSO('fake-host', 'firefly@google.com')) gerrit_util.ShouldUseSSO('fake-host.googlesource.com',
'firefly@google.com'))
def testBadHost(self):
self.assertFalse(
gerrit_util.ShouldUseSSO('fake-host.coreboot.org',
'firefly@google.com'))
def testEmptyEmail(self): def testEmptyEmail(self):
self.assertTrue(gerrit_util.ShouldUseSSO('fake-host', '')) self.assertTrue(
gerrit_util.ShouldUseSSO('fake-host.googlesource.com', ''))
def testGoogleEmail(self): def testGoogleEmail(self):
self.assertTrue( self.assertTrue(
gerrit_util.ShouldUseSSO('fake-host', 'firefly@google.com')) gerrit_util.ShouldUseSSO('fake-host.googlesource.com',
'firefly@google.com'))
def testGmail(self): def testGmail(self):
self.assertFalse( self.assertFalse(
gerrit_util.ShouldUseSSO('fake-host', 'firefly@gmail.com')) gerrit_util.ShouldUseSSO('fake-host.googlesource.com',
'firefly@gmail.com'))
@mock.patch('gerrit_util.GetAccountEmails', @mock.patch('gerrit_util.GetAccountEmails',
return_value=[{ return_value=[{
@ -776,8 +786,11 @@ class ShouldUseSSOTest(unittest.TestCase):
}]) }])
def testLinkedChromium(self, email): def testLinkedChromium(self, email):
self.assertTrue( self.assertTrue(
gerrit_util.ShouldUseSSO('fake-host', 'firefly@chromium.org')) gerrit_util.ShouldUseSSO('fake-host.googlesource.com',
email.assert_called_with('fake-host', 'self', authenticator=mock.ANY) 'firefly@chromium.org'))
email.assert_called_with('fake-host.googlesource.com',
'self',
authenticator=mock.ANY)
@mock.patch('gerrit_util.GetAccountEmails', @mock.patch('gerrit_util.GetAccountEmails',
return_value=[{ return_value=[{
@ -785,8 +798,11 @@ class ShouldUseSSOTest(unittest.TestCase):
}]) }])
def testUnlinkedChromium(self, email): def testUnlinkedChromium(self, email):
self.assertFalse( self.assertFalse(
gerrit_util.ShouldUseSSO('fake-host', 'firefly@chromium.org')) gerrit_util.ShouldUseSSO('fake-host.googlesource.com',
email.assert_called_with('fake-host', 'self', authenticator=mock.ANY) 'firefly@chromium.org'))
email.assert_called_with('fake-host.googlesource.com',
'self',
authenticator=mock.ANY)
if __name__ == '__main__': if __name__ == '__main__':

Loading…
Cancel
Save