Refactor login

server/registration/mutations_public.py

@@ -12,7 +12,7 @@ from django.contrib.auth import login
 from graphene import relay
 
 from core.hep_client import HepClient, HepClientException, HepClientUnauthorizedException
-from users.user_signup_login_handler import handle_user_and_verify_products
+from users.user_signup_login_handler import handle_user_and_verify_products, UNKNOWN_ERROR
 
 
 class RegistrationError(graphene.ObjectType):
@@ -42,16 +42,18 @@ class Registration(relay.ClientIDMutation):
 
         user, status_msg = handle_user_and_verify_products(user_data)
 
+        if user:
+            login(info.context, user)
+
         if status_msg:
             return cls.return_registration_msg(status_msg)
 
-        login(info.context, user)
         return cls(success=True, errors=[], message='success')
 
     @classmethod
     def return_registration_msg(cls, message):
         # even if the user has no valid license treat it like a success
-        if message == 'unknown_error':
+        if message == UNKNOWN_ERROR:
             error = RegistrationError(field=message)
             return cls(success=False, errors=[error], message='')
 

server/users/managers.py

@@ -100,10 +100,10 @@ class UserManager(DjangoUserManager):
         return user
 
     def create_user_from_hep(self, user_data):
-        user = self.user = self._create_user_with_random_password_no_save(
+        user = self._create_user_with_random_password_no_save(
             user_data['firstname'], user_data['lastname'], user_data['email'])
 
         user.hep_id = user_data['id']
-        user.hep_gruop_id = user_data['group_id']
+        user.hep_group_id = user_data['group_id']
         user.save()
         return user

server/users/mutations_public.py

@@ -14,7 +14,7 @@ from django.contrib.auth import authenticate, login
 from graphene import relay
 
 from core.hep_client import HepClient, HepClientUnauthorizedException, HepClientException
-from users.user_signup_login_handler import handle_user_and_verify_products
+from users.user_signup_login_handler import handle_user_and_verify_products, UNKNOWN_ERROR, EMAIL_NOT_VERIFIED
 
 
 class LoginError(graphene.ObjectType):
@@ -27,6 +27,7 @@ class Login(relay.ClientIDMutation):
         password_input = graphene.String()
 
     success = graphene.Boolean()
+    message = graphene.String()
     errors = graphene.List(LoginError)  # todo: change for consistency
 
     @classmethod
@@ -38,7 +39,7 @@ class Login(relay.ClientIDMutation):
             password = kwargs.get('password_input')
             user = authenticate(username=username, password=password)
             if user is None:
-                return cls.return_login_error('invalid_credentials')
+                return cls.return_login_message('invalid_credentials')
 
         else:
             hep_client = HepClient()
@@ -47,22 +48,26 @@ class Login(relay.ClientIDMutation):
             try:
                 user_data = hep_client.customer_me(token)
             except HepClientUnauthorizedException:
-                return cls.return_login_error('invalid_credentials')
+                return cls.return_login_message('invalid_credentials')
             except HepClientException:
-                return cls.return_login_error('unknown_error')
+                return cls.return_login_message(UNKNOWN_ERROR)
 
-            user, error_msg = handle_user_and_verify_products(user_data)
+            user, status_msg = handle_user_and_verify_products(user_data)
 
-            if error_msg:
+            if user and status_msg != EMAIL_NOT_VERIFIED:
-                return cls.return_login_error(error_msg)
+                login(info.context, user)
 
-        login(info.context, user)
+            if status_msg:
-        return cls(success=True, errors=[])
+                return cls.return_login_message(status_msg)
+
+        return cls(success=True, errors=[], message='success')
 
     @classmethod
-    def return_login_error(cls, message):
+    def return_login_message(cls, message):
-        error = LoginError(field=message)
+        if message == EMAIL_NOT_VERIFIED or message == UNKNOWN_ERROR or message == 'invalid_credentials':
-        return cls(success=False, errors=[error])
+            error = LoginError(field=message)
+            return cls(success=False, errors=[error], message='')
+        return cls(success=True, errors=[], message=message)
 
 
 class UserMutations:

server/users/tests/test_login.py

@@ -50,6 +50,7 @@ class PasswordResetTests(TestCase):
             mutation Login($input: LoginInput!){
               login(input: $input) {
                 success
+                message
                 errors {
                   field
                 }
@@ -118,7 +119,7 @@ class PasswordResetTests(TestCase):
 
     @patch.object(requests, 'post', return_value=MockResponse(401))
     def test_user_with_no_login_cannot_login(self, post_mock):
-        result = self.make_login_mutation(ME_DATA['email'], TOKEN)
+        result = self.make_login_mutation('some@some.ch', 'some')
 
         self.assertFalse(result.get('data').get('login').get('success'))
         self.assertEqual(result.get('data').get('login').get('errors')[0].get('field'), 'invalid_credentials')
@@ -136,27 +137,29 @@ class PasswordResetTests(TestCase):
     @patch.object(HepClient, 'myskillbox_product_for_customer', return_value=None)
     @patch.object(HepClient, 'customer_me', return_value=ME_DATA)
     @patch.object(HepClient, 'fetch_admin_token', return_value={'token': 'AABBCCDDEE**44566'})
-    def test_user_cannot_login_without_license(self, me_mock, product_mock, admin_token_mock):
+    def test_user_can_login_without_license(self, me_mock, product_mock, admin_token_mock):
         result = self.make_login_mutation(self.user.email, TOKEN)
 
-        self.assertFalse(result.get('data').get('login').get('success'))
+        self.assertTrue(result.get('data').get('login').get('success'))
-        self.assertEqual(result.get('data').get('login').get('errors')[0].get('field'), 'no_valid_license')
+        self.assertEqual(result.get('data').get('login').get('message'), 'no_valid_license')
+        self.assertTrue(self.user.is_authenticated)
 
     @patch.object(HepClient, 'myskillbox_product_for_customer', return_value=None)
     @patch.object(HepClient, 'customer_me', return_value=ME_DATA)
     @patch.object(HepClient, 'fetch_admin_token', return_value={'token': 'AABBCCDDEE**44566'})
-    def test_user_cannot_login_local_license_invalid(self, product_mock, me_mock, admin_token_mock):
+    def test_user_can_login_local_license_invalid(self, product_mock, me_mock, admin_token_mock):
         now = timezone.now()
         expiry_date = now - timedelta(1)
         LicenseFactory(expire_date=expiry_date, licensee=self.user, for_role=self.teacher_role).save()
 
         result = self.make_login_mutation(self.user.email, TOKEN)
 
-        self.assertFalse(result.get('data').get('login').get('success'))
+        self.assertTrue(result.get('data').get('login').get('success'))
-        self.assertEqual(result.get('data').get('login').get('errors')[0].get('field'), 'no_valid_license')
+        self.assertEqual(result.get('data').get('login').get('message'), 'no_valid_license')
+        self.assertTrue(self.user.is_authenticated)
 
     @patch.object(HepClient, 'customer_me', return_value=NOT_CONFIRMED_ME)
-    def test_user_cannot_login_with_unconfirmed_email(self, me_mock):
+    def test_user_can_login_with_unconfirmed_email(self, me_mock):
         result = self.make_login_mutation(self.user.email, TOKEN)
 
         self.assertFalse(result.get('data').get('login').get('success'))

server/users/user_signup_login_handler.py

@@ -15,6 +15,10 @@ from registration.models import License
 from users.models import User, UserRole, Role, SchoolClass
 
 
+EMAIL_NOT_VERIFIED = 'email_not_verified'
+UNKNOWN_ERROR = 'unknown_error'
+NO_VALID_LICENSE = 'no_valid_license'
+
 def handle_user_and_verify_products(user_data):
 
     hep_client = HepClient()
@@ -28,9 +32,9 @@ def handle_user_and_verify_products(user_data):
 
     try:
         if not hep_client.is_email_verified(user_data):
-            return user, 'email_not_verified'
+            return user, EMAIL_NOT_VERIFIED
     except HepClientException:
-        return user, 'unknown_error'
+        return user, UNKNOWN_ERROR
 
     try:
         license = License.objects.get(licensee=user)
@@ -40,14 +44,14 @@ def handle_user_and_verify_products(user_data):
             admin_token = AdminData.objects.get_admin_token()
             product = hep_client.myskillbox_product_for_customer(admin_token, user.hep_id)
         except HepClientException:
-            return user, 'unknown_error'
+            return user, UNKNOWN_ERROR
 
         if product:
             license = License.objects.create_license_for_role(user, product['activated'],
                                                               product['raw'], product['edition'])
             # todo handle no license case
         else:
-            return user, 'no_valid_license'
+            return user, NO_VALID_LICENSE
 
         UserRole.objects.create_role_for_user(user, license.for_role.key)
 
@@ -55,6 +59,6 @@ def handle_user_and_verify_products(user_data):
             SchoolClass.create_default_group_for_teacher(user)
 
     if not license.is_valid():
-        return user, 'no_valid_license'
+        return user, NO_VALID_LICENSE
 
     return user, None