Improve authentication messages to users (#294)

Parent issue: sequentech/epi#28

iam/api/tests.py

@@ -30,7 +30,7 @@
 from .models import ACL, AuthEvent, Action, BallotBox, TallySheet, SuccessfulLogin
 from authmethods.models import Code, MsgLog, OneTimeLink
 from authmethods import m_sms_otp
-from utils import HMACToken, verifyhmac, reproducible_json_dumps
+from utils import HMACToken, verifyhmac, reproducible_json_dumps, ErrorCodes
 from authmethods.utils import get_cannonical_tlf, get_user_code
 
 def flush_db_load_fixture(ffile="initial.json"):
@@ -1863,7 +1863,7 @@ def test_authenticate_authevent_email_invalid_code(self):
         response = c.authenticate(self.aeid, data)
         self.assertEqual(response.status_code, 400)
         r = parse_json_response(response)
-        self.assertEqual(r['error_codename'], 'invalid_credentials')
+        self.assertEqual(r['error_codename'], ErrorCodes.INVALID_CODE)
 
     @override_settings(**override_celery_data)
     def test_authenticate_authevent_email_fields(self):
@@ -2332,7 +2332,7 @@ def test_authenticate_authevent_sms_invalid_code(self):
         response = c.authenticate(self.aeid, data)
         self.assertEqual(response.status_code, 400)
         r = parse_json_response(response)
-        self.assertEqual(r['error_codename'], 'invalid_credentials')
+        self.assertEqual(r['error_codename'], ErrorCodes.INVALID_CODE)
 
     def _test_authenticate_authevent_sms_fields(self):
         c = JClient()
@@ -3932,7 +3932,7 @@ def test_census_delete_ok(self):
         )
         self.assertEqual(response.status_code, 400)
         r = parse_json_response(response)
-        self.assertEqual(r['error_codename'], 'invalid_credentials')
+        self.assertEqual(r['error_codename'], ErrorCodes.USER_NOT_FOUND)
 
         # voter does not exist in database anymore
         self.assertEqual(User.objects.filter(id=self.census_user_id).count(), 0)
@@ -4028,7 +4028,7 @@ def test_census_delete_voted_ok(self):
         )
         self.assertEqual(response.status_code, 400)
         r = parse_json_response(response)
-        self.assertEqual(r['error_codename'], 'invalid_credentials')
+        self.assertEqual(r['error_codename'], ErrorCodes.USER_NOT_FOUND)
 
         # voter does not exist in database anymore
         self.assertEqual(User.objects.filter(id=self.census_user_id).count(), 0)
@@ -4132,7 +4132,7 @@ def test_activation(self):
         response = c.authenticate(self.aeid, test_data.auth_email_default)
         self.assertEqual(response.status_code, 400)
         r = parse_json_response(response)
-        self.assertEqual(r['error_codename'], 'invalid_credentials')
+        self.assertEqual(r['error_codename'], ErrorCodes.USER_NOT_FOUND)
 
         # admin login
         response = c.authenticate(self.aeid, self.admin_auth_data)

iam/api/views.py

@@ -599,26 +599,41 @@ class Authenticate(View):
 
     def post(self, request, pk):
         try:
-            e = get_object_or_404(
+            auth_event = get_object_or_404(
                 AuthEvent,
                 pk=pk
             )
         except:
-            return json_response(status=400, error_codename=ErrorCodes.BAD_REQUEST)
+            return json_response(
+                status=400,
+                error_codename=ErrorCodes.AUTH_EVENT_NOT_FOUND
+            )
 
-        if (e.status != AuthEvent.STARTED and
-            e.status != AuthEvent.RESUMED and
-            e.auth_method_config.get("config", dict()).get("show_pdf") != True):
-            return json_response(status=400, error_codename=ErrorCodes.BAD_REQUEST)
+        if (auth_event.status != AuthEvent.STARTED and
+            auth_event.status != AuthEvent.RESUMED and
+            auth_event.auth_method_config.get("config", dict()).get("show_pdf") != True):
+            return json_response(
+                status=400,
+                error_codename=ErrorCodes.AUTH_EVENT_NOT_STARTED
+            )
 
         if not hasattr(request.user, 'account'):
-            error_kwargs = plugins.call("extend_auth", e)
+            error_kwargs = plugins.call("extend_auth", auth_event)
             if error_kwargs:
                 return json_response(**error_kwargs[0])
         try:
-            data = auth_authenticate(e, request)
-        except:
-            return json_response(status=400, error_codename=ErrorCodes.BAD_REQUEST)
+            data = auth_authenticate(auth_event, request)
+        except Exception as error:
+            LOGGER.error(
+                "Authenticate.post\n" +
+                f"req '{request}'\n" +
+                f"error '{error}'\n" +
+                f"Stack trace:\n {stack_trace_str()}"
+            )
+            return json_response(
+                status=500,
+                error_codename=ErrorCodes.INTERNAL_SERVER_ERROR
+            )
 
         if data and 'status' in data and data['status'] == 'ok':
             user = User.objects.get(username=data['username'])
@@ -627,16 +642,21 @@ def post(self, request, pk):
                 receiver=user,
                 action_name='user:authenticate',
                 event=user.userdata.event,
-                metadata=dict())
+                metadata=dict()
+            )
             action.save()
-            data["show-pdf"] = e.auth_method_config.get("config", dict()).get("show_pdf", False)
+            data["show-pdf"] = auth_event\
+                .auth_method_config\
+                .get("config", dict())\
+                .get("show_pdf", False)
 
             return json_response(data)
         else:
             return json_response(
-              status=400,
-              error_codename=data.get('error_codename'),
-              message=data.get('msg', '-'))
+                status=400,
+                error_codename=data.get('error_codename'),
+                message=data.get('msg', '-')
+            )
 authenticate = Authenticate.as_view()
 
 

iam/authmethods/m_dnie.py

@@ -15,12 +15,8 @@
 
 import json
 from . import register_method
-from utils import genhmac
 from django.shortcuts import get_object_or_404, redirect
-from django.conf import settings
-from django.contrib.auth.models import User
 from django.conf.urls import url
-from django.db.models import Q
 from django.http import Http404
 
 from authmethods.utils import check_pipeline, give_perms
@@ -116,6 +112,19 @@ class DNIE:
     )
     dni_definition = { "name": "dni", "type": "text", "required": True, "min": 2, "max": 200, "required_on_authentication": True }
 
+    def error(
+            self, msg, auth_event=None, error_codename=None, internal_error=None
+        ):
+        data = {'status': 'nok', 'msg': msg, 'error_codename': error_codename}
+        LOGGER.error(\
+            "DNIE.error\n"\
+            f"internal_error '{internal_error}'\n"\
+            f"error_codename '{error_codename}'\n"\
+            f"returning error '{data}'\n"\
+            f"auth_event '{auth_event}'\n"\
+            f"Stack trace: \n{stack_trace_str()}"
+        )
+        return data
 
     def authenticate_error(self):
         d = {'status': 'nok'}

iam/authmethods/m_email.py

@@ -18,6 +18,7 @@
 from django.conf import settings
 from django.contrib.auth.models import User
 from utils import (
+    ErrorCodes,
     constant_time_compare,
     send_codes,
     get_client_ip,
@@ -438,13 +439,17 @@ def census(self, auth_event, request):
             req, validation, auth_event, ret, stack_trace_str())
         return ret
 
-    def error(self, msg, error_codename):
+    def error(
+            self, msg, auth_event=None, error_codename=None, internal_error=None
+        ):
         data = {'status': 'nok', 'msg': msg, 'error_codename': error_codename}
         LOGGER.error(\
             "Email.error\n"\
-            "error '%r'\n"\
-            "Stack trace: \n%s",\
-            data, stack_trace_str()
+            f"internal_error '{internal_error}'\n"\
+            f"error_codename '{error_codename}'\n"\
+            f"returning error '{data}'\n"\
+            f"auth_event '{auth_event}'\n"\
+            f"Stack trace: \n{stack_trace_str()}"
         )
         return data
 
@@ -679,68 +684,74 @@ def authenticate(self, auth_event, request):
         if verified:
             if not verify_num_successful_logins(auth_event, 'Email', user, req):
                 return self.error(
-                    "Incorrect data",
-                    error_codename="invalid_credentials"
+                    ErrorCodes.CANT_VOTE_MORE_TIMES,
+                    auth_event=auth_event,
+                    error_codename=ErrorCodes.CANT_VOTE_MORE_TIMES
                 )
 
             return return_auth_data('Email', req, request, user)
 
         msg = ''
         if auth_event.parent is not None:
             msg += 'you can only authenticate to parent elections'
-            LOGGER.error(\
-                "Email.authenticate error\n"\
-                "error '%r'"\
-                "authevent '%r'\n"\
-                "request '%r'\n"\
-                "Stack trace: \n%s",\
-                msg, auth_event, req, stack_trace_str())
-            return self.error("Incorrect data", error_codename="invalid_credentials")
+            return self.error(
+                msg,
+                auth_event=auth_event,
+                error_codename=ErrorCodes.CANT_AUTHENTICATE_TO_PARENT
+            )
 
         msg += check_field_type(self.code_definition, req.get('code'), 'authenticate')
         msg += check_field_value(self.code_definition, req.get('code'), 'authenticate')
         msg += check_fields_in_request(req, auth_event, 'authenticate')
         if msg:
-            LOGGER.error(\
-                "Email.authenticate error\n"\
-                "error '%r'"\
-                "authevent '%r'\n"\
-                "request '%r'\n"\
-                "Stack trace: \n%s",\
-                msg, auth_event, req, stack_trace_str())
-            return self.error("Incorrect data", error_codename="invalid_credentials")
+            return self.error(
+                msg="",
+                internal_error=msg,
+                auth_event=auth_event,
+                error_codename=ErrorCodes.INVALID_FIELD_VALIDATION
+            )
 
         msg = check_pipeline(request, auth_event, 'authenticate')
         if msg:
-            LOGGER.error(\
-                "Email.authenticate error\n"\
-                "error '%r'\n"\
-                "authevent '%r'\n"\
-                "request '%r'\n"\
-                "Stack trace: \n%s",\
-                msg, auth_event, req, stack_trace_str())
-            return self.error("Incorrect data", error_codename="invalid_credentials")
+            return self.error(
+                msg="",
+                internal_error=msg,
+                auth_event=auth_event,
+                error_codename=ErrorCodes.PIPELINE_INVALID_CREDENTIALS
+            )
+        msg = ""
 
         try:
             q = get_base_auth_query(auth_event)
             q = get_required_fields_on_auth(req, auth_event, q)
             user = User.objects.get(q)
+        except Exception as error:
+            msg += f"can't find user with query: `{str(q)}`\nexception: `{error}`\n"
+            return self.error(
+                msg="",
+                internal_error=msg,
+                auth_event=auth_event,
+                error_codename=ErrorCodes.USER_NOT_FOUND
+            )
+        try:
             otp_field_code = post_verify_fields_on_auth(user, req, auth_event)
-        except:
-            LOGGER.error(\
-                "Email.authenticate error\n"\
-                "user not found with these characteristics: \n"\
-                "authevent '%r'\n"\
-                "is_active True\n"\
-                "request '%r'\n"\
-                "Stack trace: \n%s",\
-                auth_event, req, stack_trace_str())
-            return self.error("Incorrect data", error_codename="invalid_credentials")
+        except Exception as error:
+            msg += f"exception: `{error}`\n"
+            return self.error(
+                msg="",
+                internal_error=msg,
+                auth_event=auth_event,
+                error_codename=ErrorCodes.INVALID_PASSWORD_OR_CODE
+            )
 
         user_auth_event = user.userdata.event
 
         if not verify_num_successful_logins(user_auth_event, 'Email', user, req):
-            return self.error("Incorrect data", error_codename="invalid_credentials")
+            return self.error(
+                ErrorCodes.CANT_VOTE_MORE_TIMES,
+                auth_event=auth_event,
+                error_codename=ErrorCodes.CANT_VOTE_MORE_TIMES
+            )
 
         if otp_field_code is not None:
             code = otp_field_code
@@ -750,31 +761,22 @@ def authenticate(self, auth_event, request):
                 timeout_seconds=None
             )
         if not code:
-            LOGGER.error(\
-                "Email.authenticate error\n"\
-                "Code not found on db for user '%r'\n"\
-                "and code '%r'\n"\
-                "authevent '%r'\n"\
-                "request '%r'\n"\
-                "Stack trace: \n%s",\
-                user.userdata,\
-                req.get('code').upper(),\
-                auth_event, req, stack_trace_str())
-            return self.error("Incorrect data", error_codename="invalid_credentials")
-
-        if not constant_time_compare(req.get('code').upper(), code.code):  
-            LOGGER.error(\
-                "Email.authenticate error\n"\
-                "Code mismatch for user '%r'\n"\
-                "Code received '%r'\n"\
-                "and latest code in the db for the user '%r'\n"\
-                "authevent '%r'\n"\
-                "request '%r'\n"\
-                "Stack trace: \n%s",\
-                user.userdata, req.get('code').upper(), code.code, auth_event, req,\
-                stack_trace_str())
+            msg += f"code not found in the database for user `{user.userdata}` and requested code `{req.get('code').upper()}`\n"
+            return self.error(
+                msg="",
+                internal_error=msg,
+                auth_event=auth_event,
+                error_codename=ErrorCodes.INVALID_CODE
+            )
 
-            return self.error("Incorrect data", error_codename="invalid_credentials")
+        if not constant_time_compare(req.get('code').upper(), code.code):
+            msg += f"code mismatch for user `{user.userdata}`: [dbcode = `{code.code}`] != [requested code = `{req.get('code').upper()}`]\n"
+            return self.error(
+                msg="",
+                internal_error=msg,
+                auth_event=auth_event,
+                error_codename=ErrorCodes.INVALID_CODE
+            )
 
         return return_auth_data('Email', req, request, user)