From af1a85841bfe7083a6f38c20e10913420e2e1682 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Fri, 15 Dec 2017 04:00:08 +0000 Subject: [PATCH 1/8] check python-requests-kerberos version before gssapi login relates: #747 --- koji/__init__.py | 12 ++++++++++-- tests/test_lib/test_gssapi.py | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/koji/__init__.py b/koji/__init__.py index a6147e2e..787c376e 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -64,8 +64,10 @@ import random import re import requests try: + import requests_kerberos from requests_kerberos import HTTPKerberosAuth except ImportError: #pragma: no cover + requests_kerberos = None HTTPKerberosAuth = None import rpm import shutil @@ -2206,7 +2208,7 @@ class ClientSession(object): return host def gssapi_login(self, principal=None, keytab=None, ccache=None, proxyuser=None): - if not HTTPKerberosAuth: + if not HTTPKerberosAuth or not requests_kerberos: raise PythonImportError( "Please install python-requests-kerberos to use GSSAPI." ) @@ -2234,7 +2236,13 @@ class ClientSession(object): old_env['KRB5CCNAME'] = os.environ.get('KRB5CCNAME') os.environ['KRB5CCNAME'] = ccache if principal: - kwargs['principal'] = principal + if int(requests_kerberos.__version__.split('.')[0]) == 0 \ + and int(requests_kerberos.__version__.split('.')[1]) < 9: + e_str = 'version of python-requests-kerberos(%s) should >= 0.9.0' % requests_kerberos.__version__ + self.logger.debug(e_str) + raise PythonImportError(e_str) + else: + kwargs['principal'] = principal self.opts['auth'] = HTTPKerberosAuth(**kwargs) try: # Depending on the server configuration, we might not be able to diff --git a/tests/test_lib/test_gssapi.py b/tests/test_lib/test_gssapi.py index 4927f969..62c6d550 100644 --- a/tests/test_lib/test_gssapi.py +++ b/tests/test_lib/test_gssapi.py @@ -29,7 +29,9 @@ class TestGSSAPI(unittest.TestCase): retry=False) self.assertEqual(old_environ, dict(**os.environ)) - def test_gssapi_login_keytab(self): + @mock.patch('koji.requests_kerberos.__version__', new='0.9.0') + @mock.patch('koji.HTTPKerberosAuth') + def test_gssapi_login_keytab(self, HTTPKerberosAuth_mock): principal = 'user@EXAMPLE.COM' keytab = '/path/to/keytab' ccache = '/path/to/cache' @@ -39,6 +41,19 @@ class TestGSSAPI(unittest.TestCase): retry=False) self.assertEqual(old_environ, dict(**os.environ)) + @mock.patch('koji.requests_kerberos.__version__', new='0.7.0') + def test_gssapi_login_keytab_unsupported_requests_kerberos_version(self): + principal = 'user@EXAMPLE.COM' + keytab = '/path/to/keytab' + ccache = '/path/to/cache' + old_environ = dict(**os.environ) + with self.assertRaises(koji.PythonImportError) as cm: + self.session.gssapi_login(principal, keytab, ccache) + self.assertEqual(cm.exception.args[0], + 'version of python-requests-kerberos(0.7.0) should >= 0.9.0') + self.session._callMethod.assert_not_called() + self.assertEqual(old_environ, dict(**os.environ)) + def test_gssapi_login_error(self): old_environ = dict(**os.environ) self.session._callMethod.side_effect = Exception('login failed') From c241d28c2567dc4cb233e205249e987885b7cbeb Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Thu, 21 Dec 2017 15:04:25 +0800 Subject: [PATCH 2/8] do not import `HTTPKerberosAuth` --- koji/__init__.py | 10 ++++------ tests/test_lib/test_gssapi.py | 8 ++++---- tests/test_lib_py2only/test_krbv.py | 14 ++++++++++++-- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/koji/__init__.py b/koji/__init__.py index 787c376e..c3aa872c 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -50,7 +50,7 @@ from koji.util import md5_constructor SSL_Error = None try: from OpenSSL.SSL import Error as SSL_Error -except Exception: #pragma: no cover +except Exception: # pragma: no cover # the hub imports koji, and sometimes this import fails there # see: https://cryptography.io/en/latest/faq/#starting-cryptography-using-mod-wsgi-produces-an-internalerror-during-a-call-in-register-osrandom-engine # unfortunately the workaround at the above link does not always work, so @@ -65,10 +65,8 @@ import re import requests try: import requests_kerberos - from requests_kerberos import HTTPKerberosAuth -except ImportError: #pragma: no cover +except ImportError: # pragma: no cover requests_kerberos = None - HTTPKerberosAuth = None import rpm import shutil import signal @@ -2208,7 +2206,7 @@ class ClientSession(object): return host def gssapi_login(self, principal=None, keytab=None, ccache=None, proxyuser=None): - if not HTTPKerberosAuth or not requests_kerberos: + if not requests_kerberos: raise PythonImportError( "Please install python-requests-kerberos to use GSSAPI." ) @@ -2243,7 +2241,7 @@ class ClientSession(object): raise PythonImportError(e_str) else: kwargs['principal'] = principal - self.opts['auth'] = HTTPKerberosAuth(**kwargs) + self.opts['auth'] = requests_kerberos.HTTPKerberosAuth(**kwargs) try: # Depending on the server configuration, we might not be able to # connect without client certificate, which means that the conn diff --git a/tests/test_lib/test_gssapi.py b/tests/test_lib/test_gssapi.py index 62c6d550..edd27362 100644 --- a/tests/test_lib/test_gssapi.py +++ b/tests/test_lib/test_gssapi.py @@ -17,7 +17,7 @@ class TestGSSAPI(unittest.TestCase): maxDiff = None - @mock.patch('koji.HTTPKerberosAuth', new=None) + @mock.patch('koji.requests_kerberos', new=None) def test_gssapi_disabled(self): with self.assertRaises(ImportError): self.session.gssapi_login() @@ -29,8 +29,8 @@ class TestGSSAPI(unittest.TestCase): retry=False) self.assertEqual(old_environ, dict(**os.environ)) - @mock.patch('koji.requests_kerberos.__version__', new='0.9.0') - @mock.patch('koji.HTTPKerberosAuth') + @mock.patch('requests_kerberos.__version__', new='0.9.0') + @mock.patch('requests_kerberos.HTTPKerberosAuth') def test_gssapi_login_keytab(self, HTTPKerberosAuth_mock): principal = 'user@EXAMPLE.COM' keytab = '/path/to/keytab' @@ -41,7 +41,7 @@ class TestGSSAPI(unittest.TestCase): retry=False) self.assertEqual(old_environ, dict(**os.environ)) - @mock.patch('koji.requests_kerberos.__version__', new='0.7.0') + @mock.patch('requests_kerberos.__version__', new='0.7.0') def test_gssapi_login_keytab_unsupported_requests_kerberos_version(self): principal = 'user@EXAMPLE.COM' keytab = '/path/to/keytab' diff --git a/tests/test_lib_py2only/test_krbv.py b/tests/test_lib_py2only/test_krbv.py index 13e827ed..6f9cdc67 100644 --- a/tests/test_lib_py2only/test_krbv.py +++ b/tests/test_lib_py2only/test_krbv.py @@ -1,4 +1,5 @@ from __future__ import absolute_import + import unittest # This is python-mock, not the rpm mock tool we know and love @@ -8,12 +9,21 @@ import koji class KrbVTestCase(unittest.TestCase): - @mock.patch('koji.krbV', new=None) - @mock.patch('koji.HTTPKerberosAuth', new=None) + @mock.patch('koji.requests_kerberos', new=None) def test_krbv_disabled(self): """Test that when krbV and gssapi are absent, we behave rationally""" self.assertEquals(koji.krbV, None) session = koji.ClientSession('whatever') with self.assertRaises(ImportError): session.krb_login() + + @mock.patch('koji.krbV', autospec=True) + @mock.patch('requests_kerberos.__version__', new='0.7.0') + def test_krbv_old_requests_kerberos(self, krbV_mock): + """Test that when krbV and gssapi are absent, we behave rationally""" + self.assertIsNotNone(koji.krbV) + session = koji.ClientSession('whatever') + with self.assertRaises(koji.AuthError) as cm: + session.krb_login(principal='any@somewhere.com') + self.assertEqual(cm.exception.args[0], 'cannot specify a principal without a keytab') From 6c5924b282e15ee3584e31db5e44e883057a08be Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Wed, 20 Dec 2017 04:12:31 +0000 Subject: [PATCH 3/8] replace version comparison with regexp matching --- koji/__init__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/koji/__init__.py b/koji/__init__.py index c3aa872c..6c85dcbb 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2145,7 +2145,7 @@ class ClientSession(object): sprinc = krbV.Principal(name=self._serverPrincipal(cprinc), context=ctx) ac = krbV.AuthContext(context=ctx) - ac.flags = krbV.KRB5_AUTH_CONTEXT_DO_SEQUENCE|krbV.KRB5_AUTH_CONTEXT_DO_TIME + ac.flags = krbV.KRB5_AUTH_CONTEXT_DO_SEQUENCE | krbV.KRB5_AUTH_CONTEXT_DO_TIME ac.rcache = ctx.default_rcache() # create and encode the authentication request @@ -2156,7 +2156,6 @@ class ClientSession(object): # ask the server to authenticate us (rep_enc, sinfo_enc, addrinfo) = self.callMethod('krbLogin', req_enc, proxyuser) - # Set the addrinfo we received from the server # (necessary before calling rd_priv()) # addrinfo is in (serveraddr, serverport, clientaddr, clientport) @@ -2234,8 +2233,7 @@ class ClientSession(object): old_env['KRB5CCNAME'] = os.environ.get('KRB5CCNAME') os.environ['KRB5CCNAME'] = ccache if principal: - if int(requests_kerberos.__version__.split('.')[0]) == 0 \ - and int(requests_kerberos.__version__.split('.')[1]) < 9: + if re.match(r'0[.][1-8]\b', requests_kerberos.__version__): e_str = 'version of python-requests-kerberos(%s) should >= 0.9.0' % requests_kerberos.__version__ self.logger.debug(e_str) raise PythonImportError(e_str) From 80b4bd1ad6b401b1fff329ca8cfb5f6b3876e4f7 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Wed, 20 Dec 2017 07:38:51 +0000 Subject: [PATCH 4/8] update test_gssapi.py for requests_kerberos.__version__ checking --- tests/test_lib/test_gssapi.py | 55 +++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/tests/test_lib/test_gssapi.py b/tests/test_lib/test_gssapi.py index edd27362..d121a41a 100644 --- a/tests/test_lib/test_gssapi.py +++ b/tests/test_lib/test_gssapi.py @@ -1,13 +1,14 @@ from __future__ import absolute_import -import mock + import os import unittest +import mock + import koji class TestGSSAPI(unittest.TestCase): - def setUp(self): self.session = koji.ClientSession('https://koji.example.com/kojihub', {}) self.session._callMethod = mock.MagicMock(name='_callMethod') @@ -26,33 +27,55 @@ class TestGSSAPI(unittest.TestCase): old_environ = dict(**os.environ) self.session.gssapi_login() self.session._callMethod.assert_called_once_with('sslLogin', [None], - retry=False) + retry=False) self.assertEqual(old_environ, dict(**os.environ)) - @mock.patch('requests_kerberos.__version__', new='0.9.0') @mock.patch('requests_kerberos.HTTPKerberosAuth') def test_gssapi_login_keytab(self, HTTPKerberosAuth_mock): principal = 'user@EXAMPLE.COM' keytab = '/path/to/keytab' ccache = '/path/to/cache' old_environ = dict(**os.environ) - self.session.gssapi_login(principal, keytab, ccache) - self.session._callMethod.assert_called_once_with('sslLogin', [None], - retry=False) - self.assertEqual(old_environ, dict(**os.environ)) + current_version = koji.requests_kerberos.__version__ + accepted_versions = ['0.12.0.beta1', + '0.12.0dev', + '0.12.0a1', + '0.11.0', + '0.10.0', + '0.9.0'] + for accepted_version in accepted_versions: + koji.requests_kerberos.__version__ = accepted_version + rv = self.session.gssapi_login(principal, keytab, ccache) + self.session._callMethod.assert_called_once_with('sslLogin', [None], + retry=False) + self.assertEqual(old_environ, dict(**os.environ)) + self.assertTrue(rv) + self.session._callMethod.reset_mock() + koji.requests_kerberos.__version__ = current_version - @mock.patch('requests_kerberos.__version__', new='0.7.0') def test_gssapi_login_keytab_unsupported_requests_kerberos_version(self): principal = 'user@EXAMPLE.COM' keytab = '/path/to/keytab' ccache = '/path/to/cache' old_environ = dict(**os.environ) - with self.assertRaises(koji.PythonImportError) as cm: - self.session.gssapi_login(principal, keytab, ccache) - self.assertEqual(cm.exception.args[0], - 'version of python-requests-kerberos(0.7.0) should >= 0.9.0') - self.session._callMethod.assert_not_called() - self.assertEqual(old_environ, dict(**os.environ)) + current_version = koji.requests_kerberos.__version__ + old_versions = ['0.8.0', + '0.7.0', + '0.6.1', + '0.6', + '0.5', + '0.3', + '0.2', + '0.1'] + for old_version in old_versions: + koji.requests_kerberos.__version__ = old_version + with self.assertRaises(koji.PythonImportError) as cm: + self.session.gssapi_login(principal, keytab, ccache) + self.assertEqual(cm.exception.args[0], + 'version of python-requests-kerberos(%s) should >= 0.9.0' % old_version) + self.session._callMethod.assert_not_called() + self.assertEqual(old_environ, dict(**os.environ)) + koji.requests_kerberos.__version__ = current_version def test_gssapi_login_error(self): old_environ = dict(**os.environ) @@ -60,7 +83,7 @@ class TestGSSAPI(unittest.TestCase): with self.assertRaises(koji.AuthError): self.session.gssapi_login() self.session._callMethod.assert_called_once_with('sslLogin', [None], - retry=False) + retry=False) self.assertEqual(old_environ, dict(**os.environ)) def test_gssapi_login_http(self): From 4d70642217e88a7784c908160eabc81b3446eb11 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Wed, 20 Dec 2017 07:39:57 +0000 Subject: [PATCH 5/8] update test_krbv.py for requests_kerberos.__version__ checking --- tests/test_lib_py2only/test_krbv.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/test_lib_py2only/test_krbv.py b/tests/test_lib_py2only/test_krbv.py index 6f9cdc67..d5fa4ee7 100644 --- a/tests/test_lib_py2only/test_krbv.py +++ b/tests/test_lib_py2only/test_krbv.py @@ -1,5 +1,7 @@ from __future__ import absolute_import +import base64 +import six import unittest # This is python-mock, not the rpm mock tool we know and love @@ -18,12 +20,18 @@ class KrbVTestCase(unittest.TestCase): with self.assertRaises(ImportError): session.krb_login() - @mock.patch('koji.krbV', autospec=True) + @mock.patch('koji.krbV', create=True) @mock.patch('requests_kerberos.__version__', new='0.7.0') - def test_krbv_old_requests_kerberos(self, krbV_mock): - """Test that when krbV and gssapi are absent, we behave rationally""" + @mock.patch('koji.ClientSession._serverPrincipal') + def test_krbv_old_requests_kerberos(self, _serverPrincipal_mock, krbV_mock): self.assertIsNotNone(koji.krbV) + ctx = koji.krbV.default_context.return_value + ctx.mk_req = mock.MagicMock() + ac = mock.MagicMock() + ctx.mk_req.return_value = (ac, six.b('req')) + ac.rd_priv = mock.MagicMock(return_value='session-id session-key') session = koji.ClientSession('whatever') - with self.assertRaises(koji.AuthError) as cm: - session.krb_login(principal='any@somewhere.com') - self.assertEqual(cm.exception.args[0], 'cannot specify a principal without a keytab') + session._callMethod = mock.MagicMock( + return_value=(base64.encodestring(six.b('a')), base64.encodestring(six.b('b')), [0, 1, 2, 3])) + rv = session.krb_login(principal='any@SOMEWHERE.COM', keytab='/path/to/keytab') + self.assertTrue(rv) From 0d2588b9ad2b9512608239960fe01045e997fee9 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Fri, 22 Dec 2017 00:17:09 +0800 Subject: [PATCH 6/8] remove duplicate logging --- koji/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/koji/__init__.py b/koji/__init__.py index 6c85dcbb..943a44e7 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2234,9 +2234,9 @@ class ClientSession(object): os.environ['KRB5CCNAME'] = ccache if principal: if re.match(r'0[.][1-8]\b', requests_kerberos.__version__): - e_str = 'version of python-requests-kerberos(%s) should >= 0.9.0' % requests_kerberos.__version__ - self.logger.debug(e_str) - raise PythonImportError(e_str) + raise PythonImportError( + 'version of python-requests-kerberos(%s) should >= 0.9.0' % requests_kerberos.__version__ + ) else: kwargs['principal'] = principal self.opts['auth'] = requests_kerberos.HTTPKerberosAuth(**kwargs) From afd5c88afb9aea641f0c0e91c15c68cf0d341875 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Tue, 6 Feb 2018 16:06:15 +0100 Subject: [PATCH 7/8] adjust error text --- koji/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/koji/__init__.py b/koji/__init__.py index 943a44e7..8ff969a1 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2235,7 +2235,8 @@ class ClientSession(object): if principal: if re.match(r'0[.][1-8]\b', requests_kerberos.__version__): raise PythonImportError( - 'version of python-requests-kerberos(%s) should >= 0.9.0' % requests_kerberos.__version__ + 'python-requests-kerberos >= 0.9.0 required for ' + 'keytab auth' ) else: kwargs['principal'] = principal From 64ef26648f54cf0569c7e38f56aa30d059060084 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Tue, 6 Feb 2018 11:48:16 +0000 Subject: [PATCH 8/8] update test_gssapi --- tests/test_lib/test_gssapi.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test_lib/test_gssapi.py b/tests/test_lib/test_gssapi.py index d121a41a..ebdefe65 100644 --- a/tests/test_lib/test_gssapi.py +++ b/tests/test_lib/test_gssapi.py @@ -10,7 +10,8 @@ import koji class TestGSSAPI(unittest.TestCase): def setUp(self): - self.session = koji.ClientSession('https://koji.example.com/kojihub', {}) + self.session = koji.ClientSession('https://koji.example.com/kojihub', + {}) self.session._callMethod = mock.MagicMock(name='_callMethod') def tearDown(self): @@ -46,7 +47,8 @@ class TestGSSAPI(unittest.TestCase): for accepted_version in accepted_versions: koji.requests_kerberos.__version__ = accepted_version rv = self.session.gssapi_login(principal, keytab, ccache) - self.session._callMethod.assert_called_once_with('sslLogin', [None], + self.session._callMethod.assert_called_once_with('sslLogin', + [None], retry=False) self.assertEqual(old_environ, dict(**os.environ)) self.assertTrue(rv) @@ -72,7 +74,8 @@ class TestGSSAPI(unittest.TestCase): with self.assertRaises(koji.PythonImportError) as cm: self.session.gssapi_login(principal, keytab, ccache) self.assertEqual(cm.exception.args[0], - 'version of python-requests-kerberos(%s) should >= 0.9.0' % old_version) + 'python-requests-kerberos >= 0.9.0 required for ' + 'keytab auth') self.session._callMethod.assert_not_called() self.assertEqual(old_environ, dict(**os.environ)) koji.requests_kerberos.__version__ = current_version