From 8bc920c912a138fc5688972a3c66994041212f56 Mon Sep 17 00:00:00 2001
From: Yuming Zhu <yzhu@redhat.com>
Date: Nov 07 2016 03:08:15 +0000
Subject: [PATCH 1/3] [cli] unittests for login handlers


---

diff --git a/tests/test_login.py b/tests/test_login.py
index 58e5dfe..06d31f5 100644
--- a/tests/test_login.py
+++ b/tests/test_login.py
@@ -1,9 +1,67 @@
 import unittest
+import mock
+import koji
+import sys
 
+class LoginTestCase(unittest.TestCase):
+    def test_create_auth_handler(self):
+        options = mock.Mock()
+        for authtype in ['noauth', 'ssl', 'password', 'kerberos']:
+            options.authtype = authtype
+            handler = koji.create_auth_handler(options)
+            self.assertEqual(handler, koji._handler_mapping.get(authtype))
+        options.authtype = 'notavailble'
+        with self.assertRaises(koji.AuthError):
+            koji.create_auth_handler(options)
+        options.authtype = None
+        handler = koji.create_auth_handler(options)
+        self.assertIsInstance(handler, koji.DefaultAuthHandler)
 
-class MyTestCase(unittest.TestCase):
-    def test_something(self):
-        self.assertEqual(True, False)
+    def test_noauth_handler(self):
+        handler = koji.NoAuthHandler('noauth')
+        options = mock.Mock()
+        options.noauth = True
+        self.assertTrue(handler.test(options))
+        self.assertTrue(handler.login(session=None, options=options))
+
+    def test_ssl_handler(self):
+        handler = koji.SSLAuthHandler('ssl')
+        options = mock.Mock()
+        self.assertTrue(handler.test(options))
+        with mock.patch('koji.ClientSession') as MockClientSession:
+            session = MockClientSession.return_value
+            session.ssl_login.return_value = True
+            self.assertTrue(handler.login(session=session, options=options))
+            session.ssl_login.return_value = False
+            self.assertFalse(handler.login(session=session, options=options))
+            session.ssl_login.side_effect = koji.AuthError
+            with self.assertRaises(koji.AuthError):
+                handler.login(session=session, options=options)
+        options.cert = 'cert'
+        options.serverca = 'serverca'
+        options.runas = 'proxyuser'
+        self.assertEqual(handler.build_debug_info(options=options, error=koji.AuthError('errormsg')),
+                         {'proxyuser': 'proxyuser', 'serverca': 'serverca', 'cert': 'cert',
+                          'error': 'AuthError: errormsg'})
+
+    def test_password_handler(self):
+        handler = koji.PasswordAuthHandler('password')
+        options = mock.Mock()
+        options.user = None
+        self.assertFalse(bool(handler.test(options)))
+        options.user = 'user'
+        self.assertTrue(bool(handler.test(options)))
+        with mock.patch('koji.ClientSession') as MockClientSession:
+            session = MockClientSession.return_value
+            session.login.return_value = True
+            self.assertTrue(handler.login(session=session, options=options))
+            session.login.return_value = False
+            self.assertFalse(handler.login(session=session, options=options))
+            session.login.side_effect = koji.AuthError
+            with self.assertRaises(koji.AuthError):
+                handler.login(session=session, options=options)
+        self.assertEqual(handler.build_debug_info(options=options, error=koji.AuthError('errormsg')),
+                         {'user': 'user', 'error': 'AuthError: errormsg'})
 
 
 if __name__ == '__main__':

From 7844660063a5eba32379fdb3bc06843e3c55a09d Mon Sep 17 00:00:00 2001
From: Yuming Zhu <yzhu@redhat.com>
Date: Nov 07 2016 03:08:15 +0000
Subject: [PATCH 2/3] fix duplicate .idea in .gitignore


---

diff --git a/.gitignore b/.gitignore
index 540be9a..be29f6b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,4 +8,3 @@ htmlcov
 .idea
 .DS_STORE
 .vagrant
-.idea

From 96ae7d02d2ba83f0bd51fd65f917a71f94f083cf Mon Sep 17 00:00:00 2001
From: Yuming Zhu <yzhu@redhat.com>
Date: Nov 07 2016 04:46:15 +0000
Subject: [PATCH 3/3] move cli.activate_session() to koji.ClientSession.activate()


---

diff --git a/cli/koji b/cli/koji
index 06d4808..2f63bc6 100755
--- a/cli/koji
+++ b/cli/koji
@@ -7119,12 +7119,14 @@ def activate_session(session):
     """Test and login the session is applicable"""
     global options
     try:
-        koji.create_auth_handler(options=options).login(session=session, options=options)
-    except koji.AuthError as e:
+        session.activate(options)
+        if options.debug:
+            print "successfully connected to hub"
+    except koji.APIVersionError as e:
+        warn(_(e.message))
+    except Exception as e:
         error(_(e.message))
-    ensure_connection(session)
-    if options.debug:
-        print "successfully connected to hub"
+
 
 if __name__ == "__main__":
     global options
@@ -7150,7 +7152,7 @@ if __name__ == "__main__":
         value = getattr(options,k)
         if value is not None:
             session_opts[k] = value
-    session = koji.ClientSession(options.server,session_opts)
+    session = koji.ClientSession(options.server, session_opts)
     rv = 0
     try:
         rv = locals()[command].__call__(options, session, args)
diff --git a/koji/__init__.py b/koji/__init__.py
index a1bad8d..0d4a834 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -358,6 +358,14 @@ class LiveMediaError(GenericError):
     """Raised when LiveMedia Image creation fails"""
     faultCode = 1022
 
+class NetworkError(GenericError):
+    """Raised when network is broken"""
+    faultCode = 1023
+
+class APIVersionError(GenericError):
+    """Raised when API version unmatched between client and server fails"""
+    faultCode = 1024
+
 class MultiCallInProgress(object):
     """
     Placeholder class to be returned by method calls when in the process of
@@ -1847,6 +1855,26 @@ class ClientSession(object):
         self._calls = []
         self.logger = logging.getLogger('koji')
 
+    def activate(self, options):
+        """Test and login the session is applicable
+        Args: options generated by optparse
+        Returns: None, if success
+        Raises: AuthError, if auth failed, error.args[0] is the debug trace for client
+                ProtocolError, if network broken
+                APIVersionError, if API version unmatched between client and server
+        """
+        create_auth_handler(options).login(self, options)
+        self._ensure_connection()
+
+    def _ensure_connection(self):
+        try:
+            ret = self.getAPIVersion()
+            if ret != API_VERSION:
+                raise APIVersionError(
+                    "Warning: The server is at API version %d and the client is at %d" % (ret, API_VERSION))
+        except xmlrpclib.ProtocolError as e:
+            raise NetworkError("Error: Unable to connect to server", e)
+
     def _setup_connection(self):
         uri = urlparse.urlsplit(self.baseurl)
         scheme = uri[0]
@@ -2446,7 +2474,7 @@ class ClientAuthHandler(object):
         self.authtype = authtype
         self.info = {}
 
-    def login(self, session=None, options=None):
+    def login(self, session, options):
         """
         login *interface*, skip authentication by default
         return: True, if success, otherwise False. Default: True without any ops
@@ -2476,7 +2504,7 @@ class NoAuthHandler(ClientAuthHandler):
 class SSLAuthHandler(ClientAuthHandler):
     """ SSL Authentication Handler """
 
-    def login(self, session=None, options=None):
+    def login(self, session, options):
         # authenticate using SSL client cert
         return session.ssl_login(options.cert, None, options.serverca, proxyuser=options.runas)
 
@@ -2492,7 +2520,7 @@ class SSLAuthHandler(ClientAuthHandler):
 class PasswordAuthHandler(ClientAuthHandler):
     """ Normal user/pwd Authentication Handler """
 
-    def login(self, session=None, options=None):
+    def login(self, session, options):
         # authenticate using user/password
         return session.login()
 
@@ -2508,7 +2536,7 @@ class PasswordAuthHandler(ClientAuthHandler):
 class KerberosAuthHandler(ClientAuthHandler):
     """ Kerberos Authentication Handler """
 
-    def login(self, session=None, options=None):
+    def login(self, session, options):
         # authenticate via Kerberos
         try:
             if options.keytab and options.principal:
@@ -2528,13 +2556,14 @@ class KerberosAuthHandler(ClientAuthHandler):
         super(KerberosAuthHandler, self).build_debug_info(options, error)
         return self.info
 
-    def _has_krb_creds(self):
-        if not sys.modules.has_key('krbV'):
+    @staticmethod
+    def _has_krb_creds():
+        if not krbV:
             return False
         try:
             ctx = krbV.default_context()
             ccache = ctx.default_ccache()
-            princ = ccache.principal()
+            ccache.principal()
             return True
         except krbV.Krb5Error:
             return False
@@ -2548,29 +2577,28 @@ class DefaultAuthHandler(ClientAuthHandler):
     def __init__(self, options):
         self.handler_seq = _build_auth_handler_seq(options)
         if not self.handler_seq:
-            raise AuthError, "Unable to log in, no authentication methods available."
+            raise AuthError("Unable to log in, no authentication methods available.")
         self.infos = []
 
-    def login(self, session=None, options=None):
+    def login(self, session, options):
         """ Execute handler one by one """
         for (authtype, handler) in self.handler_seq:
             try:
-                if handler.login(session=session, options=options):
+                if handler.login(session, options):
                     self.authtype = handler.authtype
                     return True
                 else:
                     self.infos.append((handler.authtype, handler.build_debug_info(options)))
-            except Exception, e:
+            except Exception as e:
                 self.infos.append((handler.authtype, handler.build_debug_info(options, e)))
-        raise AuthError, self.build_debug_msg(session, options)
+        raise AuthError(self._build_debug_msg(session, options))
 
-    def build_debug_msg(self, session, options):
+    def _build_debug_msg(self, session, options):
         trace = ""
         for i, (key, info) in enumerate(self.infos):
             trace += "    - [%d] Bad Authentication via %s:\n" % (i, key)
-            for name, value in info.iteritems():
-                if not (value is None or value.strip()):
-                    value = '<EMPTYSTRING>'
+            items = sorted([(name, value) if value is None or str(value).strip() else (name, '<EMPTY>') for name, value in info.iteritems()])
+            for name, value in items:
                 trace += "        %-15s %s\n" % (name, value)
         self.info = """You are using the hub at %s
 but unable to log in, no available authentication method succeeds.
@@ -2584,8 +2612,8 @@ Type "koji --help" for help about global options,
 or check the "[%s]" section in your config files;
 or use "koji --authtype=noauth <command> ..." to skip login phase.
     --authtype=AUTHTYPE   force use of a type of authentication, options:
-                          noauth, ssl, password, or kerberos)
-""" % (session.baseurl, trace, options.profile)
+                          %s)
+""" % (session.baseurl, trace, options.profile, ', '.join(sorted(_handler_mapping.keys())))
         return self.info
 
 
@@ -2603,7 +2631,7 @@ def _get_auth_handler_pairs(keys, test=False, options=None):
         try:
             pairs.append((key, _handler_mapping[key]))
         except:
-            raise AuthError, "authtype: %s is not supported" % key
+            raise AuthError("authtype: %s is not supported" % key)
     if test:
         return [(k, v) for (k, v) in pairs if v.test(options=options)]
     return pairs
diff --git a/tests/test_login.py b/tests/test_login.py
index 06d31f5..3036e09 100644
--- a/tests/test_login.py
+++ b/tests/test_login.py
@@ -1,9 +1,14 @@
 import unittest
 import mock
+from mock import call
 import koji
-import sys
+import krbV
+import socket
+
 
 class LoginTestCase(unittest.TestCase):
+    maxDiff = None
+
     def test_create_auth_handler(self):
         options = mock.Mock()
         for authtype in ['noauth', 'ssl', 'password', 'kerberos']:
@@ -22,21 +27,20 @@ class LoginTestCase(unittest.TestCase):
         options = mock.Mock()
         options.noauth = True
         self.assertTrue(handler.test(options))
-        self.assertTrue(handler.login(session=None, options=options))
+        self.assertTrue(handler.login(None, options))
 
     def test_ssl_handler(self):
         handler = koji.SSLAuthHandler('ssl')
-        options = mock.Mock()
+        options = mock.MagicMock()
         self.assertTrue(handler.test(options))
-        with mock.patch('koji.ClientSession') as MockClientSession:
-            session = MockClientSession.return_value
-            session.ssl_login.return_value = True
-            self.assertTrue(handler.login(session=session, options=options))
-            session.ssl_login.return_value = False
-            self.assertFalse(handler.login(session=session, options=options))
-            session.ssl_login.side_effect = koji.AuthError
-            with self.assertRaises(koji.AuthError):
-                handler.login(session=session, options=options)
+        session = mock.MagicMock()
+        session.ssl_login.return_value = True
+        self.assertTrue(handler.login(session, options))
+        session.ssl_login.return_value = False
+        self.assertFalse(handler.login(session, options))
+        session.ssl_login.side_effect = koji.AuthError
+        with self.assertRaises(koji.AuthError):
+            handler.login(session, options)
         options.cert = 'cert'
         options.serverca = 'serverca'
         options.runas = 'proxyuser'
@@ -46,23 +50,145 @@ class LoginTestCase(unittest.TestCase):
 
     def test_password_handler(self):
         handler = koji.PasswordAuthHandler('password')
-        options = mock.Mock()
+        options = mock.MagicMock()
         options.user = None
         self.assertFalse(bool(handler.test(options)))
         options.user = 'user'
         self.assertTrue(bool(handler.test(options)))
-        with mock.patch('koji.ClientSession') as MockClientSession:
-            session = MockClientSession.return_value
-            session.login.return_value = True
-            self.assertTrue(handler.login(session=session, options=options))
-            session.login.return_value = False
-            self.assertFalse(handler.login(session=session, options=options))
-            session.login.side_effect = koji.AuthError
-            with self.assertRaises(koji.AuthError):
-                handler.login(session=session, options=options)
+        session = mock.MagicMock()
+        session.login.return_value = True
+        self.assertTrue(handler.login(session, options))
+        session.login.return_value = False
+        self.assertFalse(handler.login(session, options))
+        session.login.side_effect = koji.AuthError
+        with self.assertRaises(koji.AuthError):
+            handler.login(session, options)
         self.assertEqual(handler.build_debug_info(options=options, error=koji.AuthError('errormsg')),
                          {'user': 'user', 'error': 'AuthError: errormsg'})
 
+    def test_kerberos_handler(self):
+        handler = koji.KerberosAuthHandler('kerberos')
+        options = mock.MagicMock()
+        with mock.patch('koji.krbV', new=None):
+            self.assertFalse(bool(handler.test(options)))
+        with mock.patch('koji.krbV') as krbV_mock:
+            self.assertTrue(bool(handler.test(options)))
+            krbV_mock.Krb5Error = krbV.Krb5Error
+            krbV_mock.default_context.side_effect = krbV_mock.Krb5Error
+            self.assertFalse(bool(handler.test(options)))
+        session = mock.MagicMock()
+        session.krb_login.return_value = True
+        self.assertTrue(handler.login(session, options))
+        session.krb_login.assert_called_with(principal=options.principal, keytab=options.keytab,
+                                             proxyuser=options.runas)
+        session.krb_login.return_value = False
+        options.keytab = None
+        self.assertFalse(handler.login(session, options))
+        session.krb_login.assert_called_with(proxyuser=options.runas)
+        with self.assertRaises(koji.AuthError) as cm:
+            session.krb_login.side_effect = krbV.Krb5Error(10000, 'errormsg')
+            handler.login(session, options)
+        self.assertEqual(cm.exception.message, 'Kerberos authentication failed: errormsg (10000)')
+        with self.assertRaises(koji.AuthError) as cm:
+            session.krb_login.side_effect = socket.error(mock.ANY, 'errormsg')
+            handler.login(session, options)
+
+        self.assertEqual(handler.build_debug_info(options=options, error=koji.AuthError('errormsg')),
+                         {'keytab': options.keytab,
+                          'principal': options.principal,
+                          'proxyuser': options.runas,
+                          'error': 'AuthError: errormsg'})
+        self.assertEqual(cm.exception.message,
+                         'Could not connect to Kerberos authentication service: errormsg')
+
+    def test_default_handler(self):
+        with mock.patch('koji._build_auth_handler_seq', return_value=[]):
+            with self.assertRaises(koji.AuthError) as cm:
+                options = mock.MagicMock()
+                handler = koji.DefaultAuthHandler(options)
+            self.assertEqual(cm.exception.message, 'Unable to log in, no authentication methods available.')
+
+        mock_handler1 = mock.MagicMock()
+        mock_handler1.authtype = 'authtype1'
+        mock_handler2 = mock.MagicMock()
+        mock_handler2.authtype = 'authtype2'
+        mock_handler3 = mock.MagicMock()
+        mock_handler3.authtype = 'authtype3'
+        mock_handler4 = mock.MagicMock()
+        mock_handler4.authtype = 'authtype4'
+        handler_seq = [('authtype1', mock_handler1),
+                       ('authtype2', mock_handler2),
+                       ('authtype3', mock_handler3),
+                       ('authtype4', mock_handler4)]
+        mock_manager = mock.MagicMock()
+        mock_handler1.login.return_value = False
+        inner_exc = Exception()
+        mock_handler2.login.side_effect = inner_exc
+        mock_handler3.login.side_effect = [True, False]
+        mock_handler4.login.return_value = False
+
+        options = mock.MagicMock()
+        session = mock.MagicMock()
+        for (authtype, handler) in handler_seq:
+            mock_manager.attach_mock(handler, authtype + '_handler')
+        with mock.patch('koji._build_auth_handler_seq', return_value=handler_seq):
+            with mock.patch('koji.DefaultAuthHandler._build_debug_msg') as build_debug_msg_mock:
+                mock_manager.attach_mock(build_debug_msg_mock, 'build_debug_msg')
+                handler = koji.DefaultAuthHandler(options)
+                self.assertTrue(handler.login(session, options))
+                self.assertEqual(handler.authtype, 'authtype3')
+                self.assertEqual(handler.infos, [('authtype1', mock_handler1.build_debug_info.return_value),
+                                                 ('authtype2', mock_handler2.build_debug_info.return_value)])
+                with self.assertRaises(koji.AuthError):
+                    handler = koji.DefaultAuthHandler(options)
+                    handler.login(session, options)
+                self.assertEqual(handler.infos, [('authtype1', mock_handler1.build_debug_info.return_value),
+                                                 ('authtype2', mock_handler2.build_debug_info.return_value),
+                                                 ('authtype3', mock_handler3.build_debug_info.return_value),
+                                                 ('authtype4', mock_handler4.build_debug_info.return_value)])
+            self.assertListEqual(mock_manager.mock_calls,
+                                 [call.authtype1_handler.login(session, options),
+                                  call.authtype1_handler.build_debug_info(options),
+                                  call.authtype2_handler.login(session, options),
+                                  call.authtype2_handler.build_debug_info(options, inner_exc),
+                                  call.authtype3_handler.login(session, options),
+                                  call.authtype1_handler.login(session, options),
+                                  call.authtype1_handler.build_debug_info(options),
+                                  call.authtype2_handler.login(session, options),
+                                  call.authtype2_handler.build_debug_info(options, inner_exc),
+                                  call.authtype3_handler.login(session, options),
+                                  call.authtype3_handler.build_debug_info(options),
+                                  call.authtype4_handler.login(session, options),
+                                  call.authtype4_handler.build_debug_info(options),
+                                  call.build_debug_msg(session, options)])
+
+            session.baseurl = 'baseurl'
+            options.profile = 'profile'
+            handler.infos = [('authtype1', {'item1': 1, 'item2': 'str'}),
+                             ('authtype2', {'item1': 'sth', 'item3': None, 'item0': 'nth'})]
+            self.assertMultiLineEqual(handler._build_debug_msg(session, options),
+                                      """You are using the hub at baseurl
+but unable to log in, no available authentication method succeeds.
+
+Please check this trace that shows what auth methods you attempted with the errors and parameters.
+    [TRACE]:
+    - [0] Bad Authentication via authtype1:
+        item1           1
+        item2           str
+    - [1] Bad Authentication via authtype2:
+        item0           nth
+        item1           sth
+        item3           None
+
+
+(Your authentication parameters can be specified in command line or in config files.
+Type "koji --help" for help about global options,
+or check the "[profile]" section in your config files;
+or use "koji --authtype=noauth <command> ..." to skip login phase.
+    --authtype=AUTHTYPE   force use of a type of authentication, options:
+                          kerberos, noauth, password, ssl)
+""")
+
 
 if __name__ == '__main__':
     unittest.main()

