* Corrected handling of faults in authenticate (closes #9)

* Added tests about this
* Some refactoring inside the tests

Author: dploeger

pythonzimbra/tools/auth.py

@@ -14,7 +14,7 @@
 
 def authenticate(url, account, key, by='name', expires=0, timestamp=None,
                  timeout=None, request_type="xml", admin_auth=False,
-                 use_password=False):
+                 use_password=False, raise_on_error=False):
 
     """ Authenticate to the Zimbra server
 
@@ -35,7 +35,9 @@ def authenticate(url, account, key, by='name', expires=0, timestamp=None,
       use_password)
     :param use_password: The "key"-parameter holds a password. Do a password-
       based user authentication.
-    :return: The authentication token
+    :param raise_on_error: Should I raise an exception when an authentication
+      error occurs or just return None?
+    :return: The authentication token or None
     :rtype: str or None or unicode
     """
 
@@ -99,15 +101,19 @@ def authenticate(url, account, key, by='name', expires=0, timestamp=None,
 
         response = ResponseJson()
 
-    try:
+    server.send_request(auth_request, response)
 
-        server.send_request(auth_request, response)
+    if response.is_fault():
 
-    except HTTPError:
+        if raise_on_error:
 
-        # A HTTPError (which is an AuthError in most cases) occured. Simply
-        # return nothing
+            raise Exception(
+                "Cannot authenticate user: (%s) %s" % (
+                    response.get_fault_code(),
+                    response.get_fault_message()
+                )
+            )
 
         return None
 
-    return response.get_response()['AuthResponse']['authToken']['_content']
+    return response.get_response()['AuthResponse']['authToken']['_content']

tests/test_admin.py

@@ -76,7 +76,10 @@ def run_admin_test(self, request_type):
             if response.is_fault():
 
                 self.fail(
-                    "CreateAccount faulted. %s" % (response.get_response())
+                    "CreateAccount faulted. (%s) %s" % (
+                        response.get_fault_code(),
+                        response.get_fault_message()
+                    )
                 )
 
             account_id = response.get_response(
@@ -116,7 +119,10 @@ def run_admin_test(self, request_type):
             if response.is_fault():
 
                 self.fail(
-                    "Cannot remove test account. %s" % response.get_response()
+                    "Cannot remove test account: (%s) %s" % (
+                        response.get_fault_code(),
+                        response.get_fault_message()
+                    )
                 )
 
     def test_admin_xml(self):

tests/test_auth.py

@@ -39,10 +39,52 @@ def test_auth_xml(self):
                 timestamp
             )
 
-            if response is None:
+            self.assertNotEqual(
+                response,
+                None,
+                "Authentication with the configured settings "
+                "was not successful"
+            )
+
+    def test_auth_failure_xml(self):
+
+        """ Send a configured auth request with a wrong password in XML format
+        and check the result
+        """
+
+        config = get_config()
+
+        if config.getboolean('auth_test', 'enabled'):
+
+            # Run only if enabled
+
+            try:
+
+                timestamp = config.getint('auth_test', 'timestamp')
+
+            except ValueError:
+
+                # If timestamp is set to a none-integer, we'll just assume
+                # that it's unset
 
-                self.fail("Authentication with the configured settings "
-                          "was not successful")
+                timestamp = None
+
+            response = authenticate(
+                config.get('auth_test', 'url'),
+                config.get('auth_test', 'account'),
+                config.get('auth_test', 'preauthkey') + "1234",
+                config.get('auth_test', 'account_by'),
+                config.getint('auth_test', 'expires'),
+                timestamp
+            )
+
+            self.assertEqual(
+                response,
+                None,
+                "Authentication did not return 'None', but %s instead." % (
+                    response
+                )
+            )
 
     def test_auth_json(self):
 
@@ -77,10 +119,53 @@ def test_auth_json(self):
                 request_type='json'
             )
 
-            if response is None:
+            self.assertNotEqual(
+                response,
+                None,
+                "Authentication with the configured settings "
+                "was not successful"
+            )
+
+    def test_auth_failure_json(self):
+
+        """ Send a configured auth request with a wrong password in json
+        format and check the result
+        """
+
+        config = get_config()
+
+        if config.getboolean('auth_test', 'enabled'):
+
+            # Run only if enabled
+
+            try:
+
+                timestamp = config.getint('auth_test', 'timestamp')
+
+            except ValueError:
+
+                # If timestamp is set to a none-integer, we'll just assume
+                # that it's unset
+
+                timestamp = None
+
+            response = authenticate(
+                config.get('auth_test', 'url'),
+                config.get('auth_test', 'account'),
+                config.get('auth_test', 'preauthkey') + "1234",
+                config.get('auth_test', 'account_by'),
+                config.getint('auth_test', 'expires'),
+                timestamp,
+                request_type='json'
+            )
 
-                self.fail("Authentication with the configured settings "
-                          "was not successful")
+            self.assertEqual(
+                response,
+                None,
+                "Authentication did not return 'None', but %s instead." % (
+                    response
+                )
+            )
 
     def test_password_auth_xml(self):
 
@@ -103,10 +188,12 @@ def test_password_auth_xml(self):
                 request_type="xml"
             )
 
-            if response is None:
-
-                self.fail("Authentication with the configured settings "
-                          "was not ssuccessful")
+            self.assertNotEqual(
+                response,
+                None,
+                "Authentication with the configured settings "
+                "was not ssuccessful"
+            )
 
     def test_password_auth_json(self):
 
@@ -129,10 +216,12 @@ def test_password_auth_json(self):
                 request_type="json"
             )
 
-            if response is None:
-
-                self.fail("Authentication with the configured settings "
-                          "was not ssuccessful")
+            self.assertNotEqual(
+                response,
+                None,
+                "Authentication with the configured settings "
+                "was not ssuccessful"
+            )
 
     def test_admin_auth_xml(self):
 
@@ -155,10 +244,12 @@ def test_admin_auth_xml(self):
                 request_type="xml"
             )
 
-            if response is None:
-
-                self.fail("Authentication with the configured settings "
-                          "was not successful")
+            self.assertNotEqual(
+                response,
+                None,
+                "Authentication with the configured settings "
+                "was not successful"
+            )
 
     def test_admin_auth_json(self):
 
@@ -181,7 +272,9 @@ def test_admin_auth_json(self):
                 request_type="json"
             )
 
-            if response is None:
-
-                self.fail("Authentication with the configured settings "
-                          "was not successful")
+            self.assertNotEqual(
+                response,
+                None,
+                "Authentication with the configured settings "
+                "was not successful"
+            )

tests/test_fault.py

@@ -60,7 +60,7 @@ def test_fault_non_existing_folder_json(self):
 
             comm.send_request(request, response)
 
-            self.checkResponse(
+            self.check_response(
                 response
             )
 
@@ -111,7 +111,7 @@ def test_fault_non_existing_folder_batch_json(self):
 
             comm.send_request(request, response)
 
-            self.checkResponse(
+            self.check_response(
                 response
             )
 
@@ -148,7 +148,7 @@ def test_fault_non_existing_folder_xml(self):
 
             comm.send_request(request, response)
 
-            self.checkResponse(
+            self.check_response(
                 response
             )
 
@@ -199,18 +199,19 @@ def test_fault_non_existing_folder_batch_xml(self):
 
             comm.send_request(request, response)
 
-            self.checkResponse(
+            self.check_response(
                 response
             )
 
-    def checkResponse(self, response):
+    def check_response(self, response):
 
         # Should be a fault
 
-        self.assertEqual(
-            True,
-            response.is_fault()
-        )
+        if not response.is_fault():
+
+            self.fail(
+                "Response wasn't a fault: %s" % (response.get_response())
+            )
 
         config = get_config()