Jelle refactored api v2 new user view

CHANGES.rst

@@ -7,6 +7,15 @@ Changelog of lizard-auth-server
 
 - Added newline in activation email (cosmetic reason).
 
+- New behaviour when adding users with existing username via api2/new_user/:
+  * Old: HTTP 500 statuscode with empty content
+  * New: HTTP 409 (conflict) statuscode with error message in (text) content
+
+- New behaviour when adding users with existing email via api2/new_user/:
+  * Old: HTTP 200 statuscode with userdata in (json) content
+  * New: HTTP 409 (conflict) statuscode with error message in (text) content
+
+  Note: Use api2/find_user/ to search for users.
 
 2.17 (2017-08-30)
 -----------------

lizard_auth_server/tests/test_views_api_v2.py

@@ -311,22 +311,25 @@ def test_exiting_user(self):
         form = mock.Mock()
         form.cleaned_data = self.user_data
         result = self.view.form_valid(form)
-        self.assertEquals(200, result.status_code)
+        # Should return a 409 conflict statuscode.
+        self.assertEquals(409, result.status_code)
 
     def test_exiting_user_different_case(self):
         factories.UserF(email='Pietje@Klaasje.Test.Com')
         form = mock.Mock()
         form.cleaned_data = self.user_data
         result = self.view.form_valid(form)
-        self.assertEquals(200, result.status_code)
+        # Should return a 409 conflict statucode
+        self.assertEquals(409, result.status_code)
 
     def test_exiting_user_duplicate_email(self):
         factories.UserF(email='pietje@klaasje.test.com')
         factories.UserF(email='pietje@klaasje.test.com')
         form = mock.Mock()
         form.cleaned_data = self.user_data
         result = self.view.form_valid(form)
-        self.assertEquals(200, result.status_code)
+        # Should return a 409 conflict statuscode
+        self.assertEquals(409, result.status_code)
 
     def test_duplicate_username(self):
         factories.UserF(username='pietje',

lizard_auth_server/tests/test_views_sso.py

@@ -1,4 +1,5 @@
 import uuid
+import mock
 
 from django.test import Client
 from django.test import TestCase
@@ -96,16 +97,22 @@ def authorize_and_check_redirect(self, domain, redirect):
         msg = {'request_token': request_token,
                'key': self.key,
                'domain': domain}
-        message = URLSafeTimedSerializer(self.portal.sso_secret).dumps(msg)
-        params = {'key': self.key, 'message': message}
-        response = self.client.get('/sso/authorize/', params)
-        self.assertEquals(response.status_code, 302)
 
-        msg = {'request_token': request_token, 'auth_token': auth_token}
-        message = URLSafeTimedSerializer(self.portal.sso_secret).dumps(msg)
-        expec = '{}{}?message={}'.format(redirect,
-                                         '/sso/local_login/',
-                                         message)
+        # Hardcode the test time value for both URLSafeTimedSerializers
+        hardcoded_time_value = 1511775523
+
+        with mock.patch('time.time', return_value=hardcoded_time_value):
+            message = URLSafeTimedSerializer(self.portal.sso_secret).dumps(msg)
+            params = {'key': self.key, 'message': message}
+            response = self.client.get('/sso/authorize/', params)
+            self.assertEquals(response.status_code, 302)
+
+            msg = {'request_token': request_token, 'auth_token': auth_token}
+            message = URLSafeTimedSerializer(self.portal.sso_secret).dumps(msg)
+
+            expec = '{}{}?message={}'.format(redirect,
+                                             '/sso/local_login/',
+                                             message)
 
         def _strip_after_last_dot(url):
             # After the last dot, there's some time-specific stuff that

lizard_auth_server/views_api_v2.py

@@ -423,10 +423,10 @@ def post(self, request, *args, **kwargs):
         return super(NewUserView, self).post(request, *args, **kwargs)
 
     def form_valid(self, form):
-        """Return user data of a new or existing user
+        """Try to create a new user
 
         The JWT message's content is now the form's cleaned data. So we start
-        out by extracting the contents. Then we find/create the user and
+        out by extracting the contents. Then we try to create the user and
         return it.
 
         If a new user has been created, we send an email with an activation
@@ -447,12 +447,12 @@ def form_valid(self, form):
 
         Returns:
             A dict with key ``user`` with user data like first name, last
-            name.
+            name if the user has been created.
 
             An error 400 when mandatory keys are missing from the decoded
             JWT message or when the language is unknown.
 
-            An error 409 (conflict) when the  username is already used.
+            An error 409 (conflict) when the username or email is already used.
         """
 
         portal = Portal.objects.get(sso_key=form.cleaned_data['iss'])
@@ -466,51 +466,54 @@ def form_valid(self, form):
         # Try to find the user first. You can have multiple matches.
         matching_users = User.objects.filter(
             email__iexact=form.cleaned_data['email'])
-        user = None
-        status_code = 200
+
         if matching_users:
+
+            # Return statuscode 409 (conflict) when email address is
+            # already in use.
             if len(matching_users) > 1:
                 logger.debug(
                     "More than one user found for '%s', returning the first",
                     form.cleaned_data['email'])
             user = matching_users[0]
-            logger.info("Found existing user %s, giving that one to %s",
+            logger.info("Found existing user based on email %s in %s",
                         user, portal)
 
-        if (not user and User.objects.filter(
-            username=form.cleaned_data['username']).exists()):
+            return HttpResponse("Error: Email address is already in use: %s" %
+                                form.cleaned_data['email'], status=409)
+
+        if User.objects.filter(
+            username=form.cleaned_data['username']).exists():
 
             # Return statuscode 409 (conflict) when username is already in use.
             return HttpResponse("Error: Username is already in use: %s" %
                                 form.cleaned_data['username'], status=409)
 
-        if not user:
-            # No user found by email or username
-            language = form.cleaned_data.get('language', 'en')
-            visit_url = form.cleaned_data.get('visit_url')
-
-            if language not in AVAILABLE_LANGUAGES:
-                return HttpResponseBadRequest("Language %s is not in %s" % (
-                    language,
-                    AVAILABLE_LANGUAGES))
-
-            user = self.create_and_mail_user(
-                username=form.cleaned_data['username'],
-                first_name=form.cleaned_data['first_name'],
-                last_name=form.cleaned_data['last_name'],
-                email=form.cleaned_data['email'],
-                portal=portal,
-                language=language,
-                visit_url=visit_url)
-            status_code = 201  # Created
+        # No user found by either email or username
+        # create the user and return user
+        # data in json format
+
+        language = form.cleaned_data.get('language', 'en')
+        visit_url = form.cleaned_data.get('visit_url')
+
+        if language not in AVAILABLE_LANGUAGES:
+            return HttpResponseBadRequest("Language %s is not in %s" % (
+                language,
+                AVAILABLE_LANGUAGES))
+
+        user = self.create_and_mail_user(
+            username=form.cleaned_data['username'],
+            first_name=form.cleaned_data['first_name'],
+            last_name=form.cleaned_data['last_name'],
+            email=form.cleaned_data['email'],
+            portal=portal,
+            language=language,
+            visit_url=visit_url)
 
         # Return json dump of user data with one of the following status_codes:
-        # 200 => emailadres already in use (return first matching user)
-        # 201 => new user created (return new user)
-        user_data = construct_user_data(user=user)
-        return HttpResponse(json.dumps({'user': user_data}),
-                            content_type='application/json',
-                            status=status_code)
+        return HttpResponse(
+            json.dumps({'user': construct_user_data(user=user)}),
+            content_type='application/json', status=201)
 
     def create_and_mail_user(self, username, first_name, last_name, email,
                              portal, language, visit_url):

lizard_auth_server/views_sso.py

@@ -60,7 +60,7 @@ def form_invalid(self, form):
         logger.error('Error while decrypting form: %s',
                      form.errors.as_text())
         return ErrorMessageResponse(self.request,
-                                    _('Communication error.'),
+                                    _(form.errors.as_text()),
                                     400)