From f2ad4754c10b44066a05ffe4fff1f2d1dae03716 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Thu, 13 Feb 2014 20:32:04 +1100
Subject: [PATCH 01/14] added various exceptions

---
 crowd.py | 151 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 106 insertions(+), 45 deletions(-)

diff --git a/crowd.py b/crowd.py
index 33b450b..3796582 100644
--- a/crowd.py
+++ b/crowd.py
@@ -11,6 +11,27 @@
 import requests
 
 
+class CrowdAuthFailure(Exception):
+    """A failure occurred while performing an authentication operation"""
+    pass
+
+
+class CrowdAuthDenied(Exception):
+    """Crowd server refused to perform the operation"""
+    pass
+
+
+class CrowdUserExists(Exception):
+    pass
+
+
+class CrowdError(Exception):
+    def __init__(self, message):
+        if not len(message):
+            message = "unexpected response from Crowd server"
+        Exception.__init__(self, message)
+
+
 class CrowdServer(object):
     """Crowd server authentication object.
 
@@ -93,6 +114,9 @@ def auth_ping(self):
         Returns:
             bool:
                 True if the application authentication succeeded.
+
+        Raises:
+            CrowdError: If auth ping could not be completed.
         """
 
         url = self.rest_url + "/non-existent/location"
@@ -101,10 +125,11 @@ def auth_ping(self):
         if response.status_code == 401:
             return False
         elif response.status_code == 404:
+            # A 'not found' response indicates we passed app auth
             return True
         else:
             # An error encountered - problem with the Crowd server?
-            return False
+            raise CrowdError("unidentified problem")
 
     def auth_user(self, username, password):
         """Authenticate a user account against the Crowd server.
@@ -122,19 +147,24 @@ def auth_user(self, username, password):
                 authentication was successful. See the Crowd documentation
                 for the authoritative list of attributes.
 
-            None: If authentication failed.
+            None: If received negative authentication response
+
+        Raises:
+            CrowdAuthFailure:
+                If authentication attempt failed (other than negative response)
         """
 
         response = self._post(self.rest_url + "/authentication",
                               data=json.dumps({"value": password}),
                               params={"username": username})
 
-        # If authentication failed for any reason return None
-        if not response.ok:
-            return None
-
-        # ...otherwise return a dictionary of user attributes
-        return response.json()
+        if response.code == 200:
+            return response.json()
+        elif response.code == 400:
+            j = response.json()
+            raise CrowdAuthFailure(j['reason'])
+        else:
+            raise CrowdError
 
     def get_session(self, username, password, remote="127.0.0.1"):
         """Create a session for a user.
@@ -158,7 +188,8 @@ def get_session(self, username, password, remote="127.0.0.1"):
                 authentication was successful. See the Crowd
                 documentation for the authoritative list of attributes.
 
-            None: If authentication failed.
+        Raises:
+            CrowdAuthFailure: If authentication failed.
         """
 
         params = {
@@ -175,12 +206,11 @@ def get_session(self, username, password, remote="127.0.0.1"):
                               data=json.dumps(params),
                               params={"expand": "user"})
 
-        # If authentication failed for any reason return None
-        if not response.ok:
-            return None
-
-        # Otherwise return the user object
-        return response.json()
+        if response.status_code == 201:
+            return response.json()
+        elif response.status_code = 400:
+            j = response.json()
+            raise CrowdAuthFailure(j['reason'])
 
     def validate_session(self, token, remote="127.0.0.1"):
         """Validate a session token.
@@ -200,7 +230,8 @@ def validate_session(self, token, remote="127.0.0.1"):
                 authentication was successful. See the Crowd
                 documentation for the authoritative list of attributes.
 
-            None: If authentication failed.
+        Raises:
+            CrowdAuthFailure: If authentication failed.
         """
 
         params = {
@@ -210,12 +241,12 @@ def validate_session(self, token, remote="127.0.0.1"):
         }
 
         url = self.rest_url + "/session/%s" % token
-        response = self._post(url, data=json.dumps(params), params={"expand": "user"})
+        response = self._post(url, data=json.dumps(params),
+                              params={"expand": "user"})
 
-        # For consistency between methods use None rather than False
-        # If token validation failed for any reason return None
+        # If token validation failed for any reason raise exception
         if not response.ok:
-            return None
+            raise CrowdAuthFailure
 
         # Otherwise return the user object
         return response.json()
@@ -230,16 +261,16 @@ def terminate_session(self, token):
         Returns:
             True: If session terminated
 
-            None: If session termination failed
+        Raises:
+            CrowdError: If authentication failed.
         """
 
         url = self.rest_url + "/session/%s" % token
         response = self._delete(url)
 
-        # For consistency between methods use None rather than False
-        # If token validation failed for any reason return None
+        # If token validation failed for any reason raise exception
         if not response.ok:
-            return None
+            raise CrowdError
 
         # Otherwise return True
         return True
@@ -260,12 +291,10 @@ def add_user(self, username, **kwargs):
         Returns:
             True: Succeeded
             False: If unsuccessful
+
+        Raises:
+            CrowdError: If authentication failed.
         """
-        # Check that mandatory elements have been provided
-        if 'password' not in kwargs:
-            raise ValueError("missing password")
-        if 'email' not in kwargs:
-            raise ValueError("missing email")
 
         components = ['username', 'password', 'first_name',
                       'last_name', 'display_name', 'active']
@@ -279,11 +308,11 @@ def add_user(self, username, **kwargs):
                     "last-name": username,
                     "display-name": username,
                     "email": kwargs["email"],
-                    "password": { "value": kwargs["password"] },
+                    "password": {"value": kwargs["password"]},
                     "active": True
                    }
-        except KeyError:
-            return ValueError
+        except KeyError as e:
+            raise ValueError("missing %s" % e.message)
 
         # Remove special case 'password'
         del(kwargs["password"])
@@ -298,11 +327,20 @@ def add_user(self, username, **kwargs):
         response = self._post(self.rest_url + "/user",
                               data=json.dumps(data))
 
+        # Crowd should return 201, 400 or 403
+
         if response.status_code == 201:
             return True
 
-        return False
+        if response.status_code == 400:
+            # User already exists or no password given (we checked that)
+            raise CrowdUserExists
 
+        if response.status_code == 403:
+            raise CrowdAuthDenied("application is not allowed to create "
+                                  "a new user")
+
+        raise CrowdError
 
     def get_user(self, username):
         """Retrieve information about a user
@@ -310,61 +348,84 @@ def get_user(self, username):
         Returns:
             dict: User information
 
-            None: If no user or failure occurred
+            None: If no such user
+
+        Raises:
+            CrowdError: If unexpected response from Crowd server
         """
 
         response = self._get(self.rest_url + "/user",
                              params={"username": username,
                                      "expand": "attributes"})
 
-        if not response.ok:
+        if response.status_code == 200:
+            return response.json()
+
+        if response.status_code == 404:
             return None
 
-        return response.json()
+        raise CrowdError
 
     def get_groups(self, username):
-        """Retrieves a list of group names that have <username> as a direct member.
+        """Retrieves a list of group names that have <username> as a
+        direct member.
 
         Returns:
             list:
                 A list of strings of group names.
+
+            None: If user not found
+
+        Raises:
+            CrowdError: If unexpected response from Crowd server
         """
 
         response = self._get(self.rest_url + "/user/group/direct",
                              params={"username": username})
 
-        if not response.ok:
+        if response.status_code == 200:
+            return [g['name'] for g in response.json()['groups']]
+
+        if response.status_code == 404:
             return None
 
-        return [g['name'] for g in response.json()['groups']]
+        raise CrowdError
 
     def get_nested_groups(self, username):
-        """Retrieve a list of all group names that have <username> as a direct or indirect member.
+        """Retrieve a list of all group names that have <username> as
+        a direct or indirect member.
 
         Args:
             username: The account username.
 
-
         Returns:
             list:
                 A list of strings of group names.
+
+            None: If user not found
+
+        Raises:
+            CrowdError: If unexpected response from Crowd server
         """
 
         response = self._get(self.rest_url + "/user/group/nested",
                              params={"username": username})
 
-        if not response.ok:
+        if response.status_code == 200:
+            return [g['name'] for g in response.json()['groups']]
+
+        if response.status_code == 404:
             return None
 
-        return [g['name'] for g in response.json()['groups']]
+        raise CrowdError
 
     def get_nested_group_users(self, groupname):
-        """Retrieves a list of all users that directly or indirectly belong to the given groupname.
+        """Retrieves a list of all users that directly or indirectly
+        belong to the given groupname.
 
         Args:
             groupname: The group name.
 
-
         Returns:
             list:
                 A list of strings of user names.

From 57788cc17cdbe459030b8a5bd6d37b61149183a4 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Thu, 13 Feb 2014 21:26:09 +1100
Subject: [PATCH 02/14] updated unittests

---
 crowd.py                 | 26 ++++++++++-----------
 tests/crowdserverstub.py |  2 +-
 tests/unittests.py       | 49 ++++++++++++++++++++++------------------
 3 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/crowd.py b/crowd.py
index 3796582..c29d95f 100644
--- a/crowd.py
+++ b/crowd.py
@@ -26,8 +26,9 @@ class CrowdUserExists(Exception):
 
 
 class CrowdError(Exception):
-    def __init__(self, message):
-        if not len(message):
+    """Generic exception when unexpected response encountered"""
+    def __init__(self, message=None):
+        if not message:
             message = "unexpected response from Crowd server"
         Exception.__init__(self, message)
 
@@ -158,11 +159,11 @@ def auth_user(self, username, password):
                               data=json.dumps({"value": password}),
                               params={"username": username})
 
-        if response.code == 200:
+        if response.status_code == 200:
             return response.json()
-        elif response.code == 400:
+        elif response.status_code == 400:
             j = response.json()
-            raise CrowdAuthFailure(j['reason'])
+            raise CrowdAuthFailure(j['message'])
         else:
             raise CrowdError
 
@@ -206,11 +207,12 @@ def get_session(self, username, password, remote="127.0.0.1"):
                               data=json.dumps(params),
                               params={"expand": "user"})
 
-        if response.status_code == 201:
+        # TODO check correctness of status codes against live server
+        if response.status_code == 201 or response.status_code == 200:
             return response.json()
-        elif response.status_code = 400:
+        elif response.status_code == 400:
             j = response.json()
-            raise CrowdAuthFailure(j['reason'])
+            raise CrowdAuthFailure(j['message'])
 
     def validate_session(self, token, remote="127.0.0.1"):
         """Validate a session token.
@@ -268,13 +270,11 @@ def terminate_session(self, token):
         url = self.rest_url + "/session/%s" % token
         response = self._delete(url)
 
-        # If token validation failed for any reason raise exception
-        if not response.ok:
+        if response.status_code == 204:
+            return True
+        else:
             raise CrowdError
 
-        # Otherwise return True
-        return True
-
     def add_user(self, username, **kwargs):
         """Add a user to the directory
 
diff --git a/tests/crowdserverstub.py b/tests/crowdserverstub.py
index 031a2c6..a923230 100644
--- a/tests/crowdserverstub.py
+++ b/tests/crowdserverstub.py
@@ -298,7 +298,7 @@ def _get_session(self):
         # Either user may authenticate, used an invalid password,
         # or user does not exist.
         if user_authenticated:
-            response_code = 200
+            response_code = 201
             token = create_session(username, remote)
             response = {
                 "token": token,
diff --git a/tests/unittests.py b/tests/unittests.py
index 9261244..e336d2c 100644
--- a/tests/unittests.py
+++ b/tests/unittests.py
@@ -101,13 +101,15 @@ def testAuthUserValid(self):
 
     def testAuthUserInvalidUser(self):
         """User may not authenticate with invalid username"""
-        result = self.crowd.auth_user('invaliduser', 'xxxxx')
-        self.assertIs(result, None)
+        def f():
+            result = self.crowd.auth_user('invaliduser', 'xxxxx')
+        self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testAuthUserInvalidPass(self):
         """User may not authenticate with invalid password"""
-        result = self.crowd.auth_user(USER, 'xxxxx')
-        self.assertIs(result, None)
+        def f():
+            result = self.crowd.auth_user(USER, 'xxxxx')
+        self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testCreateSessionValidUser(self):
         """User may create a session with valid credentials"""
@@ -116,26 +118,29 @@ def testCreateSessionValidUser(self):
 
     def testCreateSessionInvalidUser(self):
         """User may not create a session with invalid username"""
-        result = self.crowd.get_session('invaliduser', 'xxxxx')
-        self.assertIs(result, None)
+        #def f():
+        #    result = self.crowd.get_session('invaliduser', 'xxxxx')
+        #self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testCreateSessionInvalidPass(self):
         """User may not create a session with invalid password"""
-        result = self.crowd.get_session(USER, 'xxxxx')
-        self.assertIs(result, None)
+        #def f():
+        #    result = self.crowd.get_session(USER, 'xxxxx')
+        #self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testValidateSessionValidUser(self):
         """Validate a valid session token"""
-        session = self.crowd.get_session(USER, PASS)
-        token = session['token']
-        result = self.crowd.validate_session(token)
-        self.assertIsInstance(result, dict)
+        #session = self.crowd.get_session(USER, PASS)
+        #token = session['token']
+        #result = self.crowd.validate_session(token)
+        #self.assertIsInstance(result, dict)
 
     def testValidateSessionInvalidToken(self):
         """Detect invalid session token"""
-        token = '0' * 24
-        result = self.crowd.validate_session(token)
-        self.assertIs(result, None)
+        def f():
+            token = '0' * 24
+            result = self.crowd.validate_session(token)
+        self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testValidateSessionValidUserUTF8(self):
         """Validate that the library handles UTF-8 in fields properly"""
@@ -215,14 +220,14 @@ def testUserCreationSuccess(self):
         self.assertTrue(result)
 
     def testUserCreationDuplicate(self):
-        result = self.crowd.add_user('newuser1',
-                                     email='me@test.example',
-                                     password='hello')
+        def add_user():
+            result = self.crowd.add_user('newuser1',
+                                         email='me@test.example',
+                                         password='hello')
+            return result
+        result = add_user()
         self.assertTrue(result)
-        result = self.crowd.add_user('newuser1',
-                                     email='me@test.example',
-                                     password='hello')
-        self.assertFalse(result)
+        self.assertRaises(crowd.CrowdUserExists, add_user)
 
     def testUserCreationMissingPassword(self):
         def f():

From d2cbbb4d502fcfaa4676bca2420007dc48cc18e6 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Fri, 14 Feb 2014 07:54:37 +1100
Subject: [PATCH 03/14] added new exception classes

---
 crowd.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/crowd.py b/crowd.py
index c29d95f..04be860 100644
--- a/crowd.py
+++ b/crowd.py
@@ -25,6 +25,14 @@ class CrowdUserExists(Exception):
     pass
 
 
+class CrowdNoSuchUser(Exception):
+    pass
+
+
+class CrowdNoSuchGroup(Exception):
+    pass
+
+
 class CrowdError(Exception):
     """Generic exception when unexpected response encountered"""
     def __init__(self, message=None):

From b584e34919b574732a9019f72b35dd8d9a0a6342 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Fri, 14 Feb 2014 07:55:03 +1100
Subject: [PATCH 04/14] implemented add_user_to_group, remove_user_from_group

---
 crowd.py | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/crowd.py b/crowd.py
index 04be860..11ccce6 100644
--- a/crowd.py
+++ b/crowd.py
@@ -449,6 +449,74 @@ def get_nested_group_users(self, groupname):
 
         return [u['name'] for u in response.json()['users']]
 
+    def add_user_to_group(self, username, groupname):
+        """Make user a direct member of a group
+
+        Args:
+            username: The user name.
+            groupname: The group name.
+
+        Returns:
+            True: If successful
+
+        Raises:
+            CrowdNoSuchUser: The user does not exist
+            CrowdNoSuchGroup: The group does not exist
+            CrowdUserExists: The user is already a member
+            CrowdError: Unexpected response
+        """
+        response = self._post(self.rest_url + "/group/user/direct",
+                              data=json.dumps({"name": username}),
+                              params={"groupname": groupname})
+
+        if response.status_code == 201:
+            return True
+
+        if response.status_code == 400:
+            raise CrowdNoSuchUser
+
+        if response.status_code == 400:
+            raise CrowdNoSuchGroup
+
+        if response.status_code == 409:
+            raise CrowdUserExists
+
+        raise CrowdError
+
+    def remove_user_from_group(self, username, groupname):
+        """Remove user as a direct member of a group
+
+        Args:
+            username: The user name.
+            groupname: The group name.
+
+        Returns:
+            True: If successful
+
+        Raises:
+            CrowdNotFound: The user or group does not exist
+            CrowdUserExists: The user is already a member
+            CrowdError: Unexpected response
+        """
+        response = self._delete(self.rest_url + "/group/user/direct",
+                              params={"groupname": groupname,
+                                      "username": username})
+
+        if response.status_code == 204:
+            return True
+
+        if response.status_code == 404:
+            # user or group does not exist
+            j = response.json()
+            if j['message'].lower().startswith('group'):
+                raise CrowdNoSuchGroup
+            elif j['message'].lower().startswith('user'):
+                raise CrowdNoSuchUser
+            else:
+                raise CrowdError("unknown server response")
+
+        raise CrowdError
+
     def user_exists(self, username):
         """Determines if the user exists.
 

From 0fdcd97b1213fc454c51f96a4571b935fafc2961 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Fri, 14 Feb 2014 07:55:23 +1100
Subject: [PATCH 05/14] changed response handling in various methods

---
 crowd.py | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/crowd.py b/crowd.py
index 11ccce6..da24f74 100644
--- a/crowd.py
+++ b/crowd.py
@@ -133,12 +133,13 @@ def auth_ping(self):
 
         if response.status_code == 401:
             return False
-        elif response.status_code == 404:
+
+        if response.status_code == 404:
             # A 'not found' response indicates we passed app auth
             return True
-        else:
-            # An error encountered - problem with the Crowd server?
-            raise CrowdError("unidentified problem")
+
+        # An error encountered - problem with the Crowd server?
+        raise CrowdError("unidentified problem")
 
     def auth_user(self, username, password):
         """Authenticate a user account against the Crowd server.
@@ -169,11 +170,12 @@ def auth_user(self, username, password):
 
         if response.status_code == 200:
             return response.json()
-        elif response.status_code == 400:
+
+        if response.status_code == 400:
             j = response.json()
             raise CrowdAuthFailure(j['message'])
-        else:
-            raise CrowdError
+
+        raise CrowdError
 
     def get_session(self, username, password, remote="127.0.0.1"):
         """Create a session for a user.
@@ -218,10 +220,13 @@ def get_session(self, username, password, remote="127.0.0.1"):
         # TODO check correctness of status codes against live server
         if response.status_code == 201 or response.status_code == 200:
             return response.json()
-        elif response.status_code == 400:
+
+        if response.status_code == 400:
             j = response.json()
             raise CrowdAuthFailure(j['message'])
 
+        raise CrowdError
+
     def validate_session(self, token, remote="127.0.0.1"):
         """Validate a session token.
 
@@ -280,8 +285,8 @@ def terminate_session(self, token):
 
         if response.status_code == 204:
             return True
-        else:
-            raise CrowdError
+
+        raise CrowdError
 
     def add_user(self, username, **kwargs):
         """Add a user to the directory
@@ -304,8 +309,6 @@ def add_user(self, username, **kwargs):
             CrowdError: If authentication failed.
         """
 
-        components = ['username', 'password', 'first_name',
-                      'last_name', 'display_name', 'active']
         # Populate data with default and mandatory values.
         # A KeyError means a mandatory value was not provided,
         # so raise a ValueError indicating bad args.
@@ -341,7 +344,7 @@ def add_user(self, username, **kwargs):
             return True
 
         if response.status_code == 400:
-            # User already exists or no password given (we checked that)
+            # User already exists / no password given (but we checked that)
             raise CrowdUserExists
 
         if response.status_code == 403:

From a09079e4a4e953f3a2ba9679ea7447c2c0171917 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Fri, 14 Feb 2014 07:55:57 +1100
Subject: [PATCH 06/14] changed unittests to allow performing against real
 server

---
 tests/unittests.py | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/tests/unittests.py b/tests/unittests.py
index e336d2c..46c31a0 100644
--- a/tests/unittests.py
+++ b/tests/unittests.py
@@ -39,13 +39,17 @@ class testCrowdAuth(unittest.TestCase):
 
     @classmethod
     def setUpClass(cls):
-        cls.base_url = 'http://localhost:%d' % PORT
-        cls.crowd = crowd.CrowdServer(cls.base_url, APP_USER, APP_PASS)
-
-        cls.server_thread = threading.Thread(
-            target=crowdserverstub.run_server, args=(PORT,))
-        cls.server_thread.start()
+        import os
+        if 'CROWDSERVER' in os.environ:
+            cls.base_url = os.environ['CROWDSERVER']
+            cls.server_thread = None
+        else:
+            cls.base_url = 'http://localhost:%d' % PORT
+            cls.server_thread = threading.Thread(
+                target=crowdserverstub.run_server, args=(PORT,))
+            cls.server_thread.start()
 
+        cls.crowd = crowd.CrowdServer(cls.base_url, APP_USER, APP_PASS)
         crowdserverstub.add_app(APP_USER, APP_PASS)
         crowdserverstub.add_user(USER, PASS)
 
@@ -56,7 +60,8 @@ def setUpClass(cls):
     @classmethod
     def tearDownClass(cls):
         requests.get(cls.base_url + '/terminate')
-        cls.server_thread.join()
+        if cls.server_thread:
+            cls.server_thread.join()
 
     def testStubUserExists(self):
         """Check that server stub recognises user"""

From 39a4ad319fc9438063b61be4ab78d61a1406d69e Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Fri, 14 Feb 2014 07:56:34 +1100
Subject: [PATCH 07/14] various test case updates

---
 tests/unittests.py | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/tests/unittests.py b/tests/unittests.py
index 46c31a0..ccf3243 100644
--- a/tests/unittests.py
+++ b/tests/unittests.py
@@ -123,22 +123,22 @@ def testCreateSessionValidUser(self):
 
     def testCreateSessionInvalidUser(self):
         """User may not create a session with invalid username"""
-        #def f():
-        #    result = self.crowd.get_session('invaliduser', 'xxxxx')
-        #self.assertRaises(crowd.CrowdAuthFailure, f)
+        def f():
+            result = self.crowd.get_session('invaliduser', 'xxxxx')
+        self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testCreateSessionInvalidPass(self):
         """User may not create a session with invalid password"""
-        #def f():
-        #    result = self.crowd.get_session(USER, 'xxxxx')
-        #self.assertRaises(crowd.CrowdAuthFailure, f)
+        def f():
+            result = self.crowd.get_session(USER, 'xxxxx')
+        self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testValidateSessionValidUser(self):
         """Validate a valid session token"""
-        #session = self.crowd.get_session(USER, PASS)
-        #token = session['token']
-        #result = self.crowd.validate_session(token)
-        #self.assertIsInstance(result, dict)
+        session = self.crowd.get_session(USER, PASS)
+        token = session['token']
+        result = self.crowd.validate_session(token)
+        self.assertIsInstance(result, dict)
 
     def testValidateSessionInvalidToken(self):
         """Detect invalid session token"""
@@ -158,7 +158,7 @@ def testCreateSessionIdentical(self):
         """Sessions from same remote are identical"""
         session1 = self.crowd.get_session(USER, PASS, '192.168.99.99')
         session2 = self.crowd.get_session(USER, PASS, '192.168.99.99')
-        self.assertEqual(session1, session2)
+        self.assertEqual(session1['token'], session2['token'])
 
     def testCreateSessionMultiple(self):
         """User may create multiple sessions from different remote"""
@@ -176,7 +176,7 @@ def testTerminateSessionValidToken(self):
     def testTerminateSessionInvalidToken(self):
         token = '0' * 24
         result = self.crowd.terminate_session(token)
-        self.assertIs(result, None)
+        self.assertIsTrue(result)
 
     def testGetGroupsNotEmpty(self):
         crowdserverstub.add_user_to_group(USER, GROUP)
@@ -191,16 +191,20 @@ def testGetNestedGroupsNotEmpty(self):
         crowdserverstub.remove_user_from_group(USER, GROUP)
 
     def testRemoveUserFromGroup(self):
-        crowdserverstub.add_user_to_group(USER, GROUP)
-        crowdserverstub.remove_user_from_group(USER, GROUP)
+        #crowdserverstub.add_user_to_group(USER, GROUP)
+        #crowdserverstub.remove_user_from_group(USER, GROUP)
+        self.crowd.add_user_to_group(USER, GROUP)
+        self.crowd.remove_user_from_group(USER, GROUP)
         result = self.crowd.get_groups(USER)
         self.assertEqual(set(result), set([]))
 
     def testGetNestedGroupUsersNotEmpty(self):
-        crowdserverstub.add_user_to_group(USER, GROUP)
+        #crowdserverstub.add_user_to_group(USER, GROUP)
+        self.crowd.add_user_to_group(USER, GROUP)
         result = self.crowd.get_nested_group_users(GROUP)
+        #crowdserverstub.remove_user_from_group(USER, GROUP)
+        self.crowd.remove_user_from_group(USER, GROUP)
         self.assertEqual(set(result), set([USER]))
-        crowdserverstub.remove_user_from_group(USER, GROUP)
 
     def testUserExists(self):
         result = self.crowd.user_exists(USER)

From a8b56854377f4279b0690821fc7875e5e58d1152 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Sat, 15 Feb 2014 18:39:14 +1100
Subject: [PATCH 08/14] various unittest improvements and fixes

---
 tests/unittests.py | 106 +++++++++++++++++++++++++++------------------
 1 file changed, 65 insertions(+), 41 deletions(-)

diff --git a/tests/unittests.py b/tests/unittests.py
index ccf3243..16735ab 100644
--- a/tests/unittests.py
+++ b/tests/unittests.py
@@ -29,9 +29,10 @@
 print("Port {0}".format(PORT))
 APP_USER = 'testapp'
 APP_PASS = 'testpass'
-USER     = 'user1'
+USER     = 'pythoncrowdtestuser'
 PASS     = 'pass1'
-GROUP    = 'group1'
+EMAIL    = 'me@test.example'
+GROUP    = 'pythoncrowdtestgroup'
 
 
 class testCrowdAuth(unittest.TestCase):
@@ -48,35 +49,63 @@ def setUpClass(cls):
             cls.server_thread = threading.Thread(
                 target=crowdserverstub.run_server, args=(PORT,))
             cls.server_thread.start()
+            crowdserverstub.add_app(APP_USER, APP_PASS)
+            # There is a race to start the HTTP server before
+            # the unit tests begin hitting it. Sleep briefly
+            time.sleep(0.2)
 
         cls.crowd = crowd.CrowdServer(cls.base_url, APP_USER, APP_PASS)
-        crowdserverstub.add_app(APP_USER, APP_PASS)
-        crowdserverstub.add_user(USER, PASS)
 
-        # There is a race to start the HTTP server before
-        # the unit tests begin hitting it. Sleep briefly
-        time.sleep(0.2)
+        # Create user account for most tests
+        try:
+            cls.crowd.add_user(USER, password=PASS, email=EMAIL)
+        except crowd.CrowdUserExists:
+            pass
+        cls.num_users_created = 0
+        try:
+            cls.crowd.add_group(GROUP)
+        except crowd.CrowdGroupExists:
+            pass
 
     @classmethod
     def tearDownClass(cls):
-        requests.get(cls.base_url + '/terminate')
         if cls.server_thread:
+            requests.get(cls.base_url + '/terminate')
             cls.server_thread.join()
+        else:
+            # Remove users
+            try:
+                cls.crowd.remove_user(USER)
+            except:
+                pass
+            for i in xrange(0, cls.num_users_created):
+                try:
+                    cls.crowd.remove_user(USER + str(i))
+                except:
+                    pass
+            # Remove groups
+            try:
+                cls.crowd.remove_group(GROUP)
+            except:
+                pass
 
     def testStubUserExists(self):
         """Check that server stub recognises user"""
-        result = crowdserverstub.user_exists(USER)
-        self.assertTrue(result)
+        if self.server_thread:
+            result = crowdserverstub.user_exists(USER)
+            self.assertTrue(result)
 
     def testStubUserDoesNotExist(self):
         """Check that server stub does not know invalid user"""
-        result = crowdserverstub.user_exists('fakeuser')
-        self.assertFalse(result)
+        if self.server_thread:
+            result = crowdserverstub.user_exists('fakeuser')
+            self.assertFalse(result)
 
     def testStubCheckUserAuth(self):
         """Check that server stub auths our user/pass combination"""
-        result = crowdserverstub.check_user_auth(USER, PASS)
-        self.assertTrue(result)
+        if self.server_thread:
+            result = crowdserverstub.check_user_auth(USER, PASS)
+            self.assertTrue(result)
 
     def testCrowdObjectSSLVerifyTrue(self):
         """Check can create Crowd object with ssl_verify=True"""
@@ -106,15 +135,13 @@ def testAuthUserValid(self):
 
     def testAuthUserInvalidUser(self):
         """User may not authenticate with invalid username"""
-        def f():
+        with self.assertRaises(crowd.CrowdAuthFailure):
             result = self.crowd.auth_user('invaliduser', 'xxxxx')
-        self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testAuthUserInvalidPass(self):
         """User may not authenticate with invalid password"""
-        def f():
+        with self.assertRaises(crowd.CrowdAuthFailure):
             result = self.crowd.auth_user(USER, 'xxxxx')
-        self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testCreateSessionValidUser(self):
         """User may create a session with valid credentials"""
@@ -142,17 +169,20 @@ def testValidateSessionValidUser(self):
 
     def testValidateSessionInvalidToken(self):
         """Detect invalid session token"""
-        def f():
+        with self.assertRaises(crowd.CrowdAuthFailure):
             token = '0' * 24
             result = self.crowd.validate_session(token)
-        self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testValidateSessionValidUserUTF8(self):
         """Validate that the library handles UTF-8 in fields properly"""
+        username = USER + 'utf8'
+        email = u'me@test.ëxample'
+        self.crowd.add_user(username, password=PASS, email=email)
         session = self.crowd.get_session(USER, PASS)
+        print session
         token = session['token']
         result = self.crowd.validate_session(token)
-        self.assertEquals(result['user']['email'], u'%s@does.not.ëxist' % USER)
+        self.assertEquals(result['user']['email'], email)
 
     def testCreateSessionIdentical(self):
         """Sessions from same remote are identical"""
@@ -176,33 +206,29 @@ def testTerminateSessionValidToken(self):
     def testTerminateSessionInvalidToken(self):
         token = '0' * 24
         result = self.crowd.terminate_session(token)
-        self.assertIsTrue(result)
+        self.assertTrue(result)
 
     def testGetGroupsNotEmpty(self):
-        crowdserverstub.add_user_to_group(USER, GROUP)
+        self.crowd.add_user_to_group(USER, GROUP)
         result = self.crowd.get_groups(USER)
         self.assertEqual(set(result), set([GROUP]))
-        crowdserverstub.remove_user_from_group(USER, GROUP)
+        self.crowd.remove_user_from_group(USER, GROUP)
 
     def testGetNestedGroupsNotEmpty(self):
-        crowdserverstub.add_user_to_group(USER, GROUP)
+        self.crowd.add_user_to_group(USER, GROUP)
         result = self.crowd.get_nested_groups(USER)
+        self.crowd.remove_user_from_group(USER, GROUP)
         self.assertEqual(set(result), set([GROUP]))
-        crowdserverstub.remove_user_from_group(USER, GROUP)
 
     def testRemoveUserFromGroup(self):
-        #crowdserverstub.add_user_to_group(USER, GROUP)
-        #crowdserverstub.remove_user_from_group(USER, GROUP)
         self.crowd.add_user_to_group(USER, GROUP)
         self.crowd.remove_user_from_group(USER, GROUP)
         result = self.crowd.get_groups(USER)
         self.assertEqual(set(result), set([]))
 
     def testGetNestedGroupUsersNotEmpty(self):
-        #crowdserverstub.add_user_to_group(USER, GROUP)
         self.crowd.add_user_to_group(USER, GROUP)
         result = self.crowd.get_nested_group_users(GROUP)
-        #crowdserverstub.remove_user_from_group(USER, GROUP)
         self.crowd.remove_user_from_group(USER, GROUP)
         self.assertEqual(set(result), set([USER]))
 
@@ -216,26 +242,24 @@ def testUserAttributesExist(self):
         self.assertTrue('attributes' in result)
 
     def testUserAttributesReturned(self):
-        crowdserverstub.add_user('attruser', 'mypass', {'something': True})
-        result = self.crowd.get_user('attruser')
+        result = self.crowd.get_user(USER)
         self.assertIsNotNone(result)
         self.assertTrue('attributes' in result)
-        self.assertTrue('something' in result['attributes'])
+        self.assertTrue('attributes' in result['attributes'])  # Yo dawg
 
     def testUserCreationSuccess(self):
-        result = self.crowd.add_user('newuser',
-                                     email='me@test.example',
-                                     password='hello')
+        username = USER + str(self.num_users_created)
+        self.num_users_created += 1
+        print "testUserCreationSuccess"
+        print "Adding user %s" % username
+        result = self.crowd.add_user(username, password=PASS, email=EMAIL)
         self.assertTrue(result)
 
     def testUserCreationDuplicate(self):
         def add_user():
-            result = self.crowd.add_user('newuser1',
-                                         email='me@test.example',
-                                         password='hello')
+            result = self.crowd.add_user(USER, password=PASS, email=EMAIL)
             return result
-        result = add_user()
-        self.assertTrue(result)
+        # USER already created during test setup
         self.assertRaises(crowd.CrowdUserExists, add_user)
 
     def testUserCreationMissingPassword(self):

From 38126bf59138831178099cf338790ba7f04fc2f2 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Sat, 15 Feb 2014 18:40:36 +1100
Subject: [PATCH 09/14] fixed testUserCreationSuccess

---
 tests/unittests.py | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/unittests.py b/tests/unittests.py
index 16735ab..1fe149d 100644
--- a/tests/unittests.py
+++ b/tests/unittests.py
@@ -248,11 +248,9 @@ def testUserAttributesReturned(self):
         self.assertTrue('attributes' in result['attributes'])  # Yo dawg
 
     def testUserCreationSuccess(self):
-        username = USER + str(self.num_users_created)
-        self.num_users_created += 1
-        print "testUserCreationSuccess"
-        print "Adding user %s" % username
+        username = USER + "tmp"
         result = self.crowd.add_user(username, password=PASS, email=EMAIL)
+        self.crowd.remove_user(username)
         self.assertTrue(result)
 
     def testUserCreationDuplicate(self):

From 80a6233dad8b0c523d4d4bc3e38129af26e31984 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Sat, 15 Feb 2014 18:54:48 +1100
Subject: [PATCH 10/14] fixed testValidateSessionValidUserUTF8

---
 tests/unittests.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/unittests.py b/tests/unittests.py
index 1fe149d..c9c76de 100644
--- a/tests/unittests.py
+++ b/tests/unittests.py
@@ -175,13 +175,16 @@ def testValidateSessionInvalidToken(self):
 
     def testValidateSessionValidUserUTF8(self):
         """Validate that the library handles UTF-8 in fields properly"""
-        username = USER + 'utf8'
-        email = u'me@test.ëxample'
-        self.crowd.add_user(username, password=PASS, email=email)
-        session = self.crowd.get_session(USER, PASS)
-        print session
+        username = USER + "unicode"
+        email = u'ÜñÍçÔÐê'
+        try:
+            self.crowd.add_user(username, password=PASS, email=email)
+        except crowd.CrowdUserExists:
+            pass
+        session = self.crowd.get_session(username, PASS)
         token = session['token']
         result = self.crowd.validate_session(token)
+        self.crowd.remove_user(username)
         self.assertEquals(result['user']['email'], email)
 
     def testCreateSessionIdentical(self):

From c374063b382bddc9eaf10e0119b7a98c405e9d21 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Sat, 15 Feb 2014 18:59:49 +1100
Subject: [PATCH 11/14] assertions tested in 'with' blocks

---
 tests/unittests.py | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/tests/unittests.py b/tests/unittests.py
index c9c76de..b25a799 100644
--- a/tests/unittests.py
+++ b/tests/unittests.py
@@ -150,15 +150,13 @@ def testCreateSessionValidUser(self):
 
     def testCreateSessionInvalidUser(self):
         """User may not create a session with invalid username"""
-        def f():
+        with self.assertRaises(crowd.CrowdAuthFailure):
             result = self.crowd.get_session('invaliduser', 'xxxxx')
-        self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testCreateSessionInvalidPass(self):
         """User may not create a session with invalid password"""
-        def f():
+        with self.assertRaises(crowd.CrowdAuthFailure):
             result = self.crowd.get_session(USER, 'xxxxx')
-        self.assertRaises(crowd.CrowdAuthFailure, f)
 
     def testValidateSessionValidUser(self):
         """Validate a valid session token"""
@@ -257,31 +255,25 @@ def testUserCreationSuccess(self):
         self.assertTrue(result)
 
     def testUserCreationDuplicate(self):
-        def add_user():
+        with self.assertRaises(crowd.CrowdUserExists):
+            # USER already created during test setup.
+            # This is attempting to add the account again.
             result = self.crowd.add_user(USER, password=PASS, email=EMAIL)
-            return result
-        # USER already created during test setup
-        self.assertRaises(crowd.CrowdUserExists, add_user)
 
     def testUserCreationMissingPassword(self):
-        def f():
-            result = self.crowd.add_user('newuser2',
-                                         email='me@test.example')
-        self.assertRaisesRegexp(ValueError, "missing password", f)
+        with self.assertRaisesRegexp(ValueError, "missing password"):
+            result = self.crowd.add_user(USER, email=EMAIL)
 
     def testUserCreationMissingEmail(self):
-        def f():
-            result = self.crowd.add_user('newuser',
-                                         password='something')
-        self.assertRaisesRegexp(ValueError, "missing email", f)
+        with self.assertRaisesRegexp(ValueError, "missing email"):
+            result = self.crowd.add_user(USER, password=PASS)
 
     def testUserCreationInvalidParam(self):
-        def f():
+        with self.assertRaisesRegexp(ValueError, "invalid argument .*"):
             result = self.crowd.add_user('newuser',
-                                         email='me@test.example',
-                                         password='hello',
+                                         email=EMAIL,
+                                         password=PASS,
                                          invalid_param='bad argument')
-        self.assertRaisesRegexp(ValueError, "invalid argument .*", f)
 
 if __name__ == "__main__":
     unittest.main()

From 1e20942f2872683c098beff51b0f661d9d3cc0d5 Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Sat, 15 Feb 2014 19:01:43 +1100
Subject: [PATCH 12/14] added add_group

---
 crowd.py | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/crowd.py b/crowd.py
index da24f74..0dcce47 100644
--- a/crowd.py
+++ b/crowd.py
@@ -29,6 +29,10 @@ class CrowdNoSuchUser(Exception):
     pass
 
 
+class CrowdGroupExists(Exception):
+    pass
+
+
 class CrowdNoSuchGroup(Exception):
     pass
 
@@ -377,6 +381,45 @@ def get_user(self, username):
 
         raise CrowdError
 
+    def add_group(self, groupname, **kwargs):
+        """Creates a group
+
+        Returns:
+            True: The group was created
+
+        Raises:
+            CrowdGroupExists: The group already exists
+            CrowdAuthFail
+            CrowdError: If unexpected response from Crowd server
+        """
+
+        data = {
+                "name": groupname,
+                "description": groupname,
+                "active": True,
+                "type": "GROUP"
+               }
+        # Put values from kwargs into data
+        for k, v in kwargs.items():
+            if k not in data:
+                raise ValueError("invalid argument %s" % k)
+            data[k] = v
+
+        response = self._post(self.rest_url + "/group",
+                              data=json.dumps(data))
+
+        if response.status_code == 201:
+            return True
+
+        if response.status_code == 400:
+            raise CrowdGroupExists
+
+        if response.status_code == 403:
+            raise CrowdAuthFailure
+
+        raise CrowdError("status code %d" % response.status_code)
+
+
     def get_groups(self, username):
         """Retrieves a list of group names that have <username> as a
         direct member.

From 8805d837ee3298abebec220b56188b3216e138dd Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Sat, 15 Feb 2014 19:02:02 +1100
Subject: [PATCH 13/14] fixed tests for response codes

---
 crowd.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/crowd.py b/crowd.py
index 0dcce47..8920d7e 100644
--- a/crowd.py
+++ b/crowd.py
@@ -221,8 +221,7 @@ def get_session(self, username, password, remote="127.0.0.1"):
                               data=json.dumps(params),
                               params={"expand": "user"})
 
-        # TODO check correctness of status codes against live server
-        if response.status_code == 201 or response.status_code == 200:
+        if response.status_code == 201:
             return response.json()
 
         if response.status_code == 400:
@@ -521,13 +520,13 @@ def add_user_to_group(self, username, groupname):
         if response.status_code == 400:
             raise CrowdNoSuchUser
 
-        if response.status_code == 400:
+        if response.status_code == 404:
             raise CrowdNoSuchGroup
 
         if response.status_code == 409:
             raise CrowdUserExists
 
-        raise CrowdError
+        raise CrowdError("received server response %d" % response.status_code)
 
     def remove_user_from_group(self, username, groupname):
         """Remove user as a direct member of a group

From b9c88e48d994213f728c964b7ee6cf70ef69bfdf Mon Sep 17 00:00:00 2001
From: Alexander Else <aelse@else.id.au>
Date: Sat, 15 Feb 2014 19:02:20 +1100
Subject: [PATCH 14/14] added remove_user

---
 crowd.py | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/crowd.py b/crowd.py
index 8920d7e..7891f63 100644
--- a/crowd.py
+++ b/crowd.py
@@ -356,6 +356,37 @@ def add_user(self, username, **kwargs):
 
         raise CrowdError
 
+    def remove_user(self, username):
+        """Remove a user from the directory
+
+        Args:
+            username: The account username
+
+        Returns:
+            True: Succeeded
+
+        Raises:
+            CrowdNoSuchUser: If user did not exist
+            CrowdAuthDenied: If application not allowed to delete the user
+        """
+
+        response = self._delete(self.rest_url + "/user",
+                             params={"username": username})
+
+        # Crowd should return 204, 403 or 404
+
+        if response.status_code == 204:
+            return True
+
+        if response.status_code == 403:
+            raise CrowdAuthDenied("application is not allowed to delete user")
+
+        if response.status_code == 404:
+            # User did not exist
+            raise CrowdNoSuchUser
+
+        raise CrowdError
+
     def get_user(self, username):
         """Retrieve information about a user