From e3f3ad427ca176c8cdbad01e2081a5f61b371560 Mon Sep 17 00:00:00 2001
From: Liz Fong-Jones <lizf@honeycomb.io>
Date: Fri, 17 Sep 2021 12:23:32 -0700
Subject: [PATCH] avoid double-creating users

---
 helios/models.py | 67 ++++++++++++++++++++++++++++++------------------
 helios/views.py  | 10 +++-----
 2 files changed, 46 insertions(+), 31 deletions(-)

diff --git a/helios/models.py b/helios/models.py
index 368438e..f5e384e 100644
--- a/helios/models.py
+++ b/helios/models.py
@@ -16,6 +16,7 @@ import bleach
 import unicodecsv
 from django.conf import settings
 from django.db import models, transaction
+from validate_email import validate_email
 
 from helios import datatypes
 from helios import utils
@@ -768,20 +769,33 @@ class VoterFile(models.Model):
       if len(voter_fields) < 2:
         continue
 
-      return_dict = {'voter_type': voter_fields[0].strip(), 'voter_id': voter_fields[1].strip()}
+      voter_type = voter_fields[0].strip()
+      voter_id = voter_fields[1].strip()
 
+      if not voter_type in AUTH_SYSTEMS:
+        raise Exception("invalid voter type '%s' for voter id '%s'" % (voter_type, voter_id))
+
+      # default to having email be the same as voter_id
+      voter_email = voter_id
       if len(voter_fields) > 2:
-        return_dict['email'] = voter_fields[2].strip()
-      else:
-        # assume single field means the email is the same field as voter_id
-        return_dict['email'] = voter_fields[1].strip()
+        # but if it's supplied, it will be the 3rd field.
+        voter_email = voter_fields[2].strip()
+      if voter_type == "password" and not validate_email(voter_email):
+        raise Exception("invalid voter email '%s' for voter id '%s'" % (voter_email, voter_id))
 
+      # same thing for voter display name.
+      voter_name = voter_email
       if len(voter_fields) > 3:
-        return_dict['name'] = voter_fields[3].strip()
-      else:
-        return_dict['name'] = return_dict['email']
+        # which is supplied as the 4th field if known.
+        voter_name = voter_fields[3].strip()
+
+      yield {
+        'voter_type': voter_type,
+        'voter_id': voter_id,
+        'email': voter_email,
+        'name': voter_name,
+      }
 
-      yield return_dict
     if close:
       voter_stream.close()
 
@@ -794,23 +808,26 @@ class VoterFile(models.Model):
     random.shuffle(voters)
 
     for voter in voters:
-      # does voter for this user already exist
-      existing_voter = Voter.get_by_election_and_voter_id(self.election, voter['voter_id'])
-      # create the voter
-      if not existing_voter:
-          if voter['voter_type'] != 'password':
-              user, _ = User.objects.get_or_create(user_type=voter['voter_type'], user_id=voter['voter_id'], defaults = {'name': voter['voter_id'], 'info': {}, 'token': None})
+      if voter['voter_type'] == 'password':
+          # does voter for this user already exist
+          existing_voter = Voter.get_by_election_and_voter_id(self.election, voter['voter_id'])
+          if existing_voter:
+              continue
+          # create the voter
+          voter_uuid = str(uuid.uuid4())
+          new_voter = Voter(uuid=voter_uuid, user = None, voter_login_id = voter['voter_id'],
+              voter_name = voter['name'], voter_email = voter['email'], election = self.election)
+          new_voter.generate_password()
+          if election.use_voter_aliases:
+              utils.lock_row(Election, election.id)
+              alias_num = election.last_alias_num + 1
+              new_voter.alias = "V%s" % alias_num
+          new_voter.save()
+      else:
+          user, _ = User.objects.get_or_create(user_type=voter['voter_type'], user_id=voter['voter_id'], defaults = {'name': voter['voter_id'], 'info': {}, 'token': None})
+          existing_voter = Voter.get_by_election_and_user(self.election, user)
+          if not existing_voter:
               Voter.register_user_in_election(user, self.election)
-          else:
-              voter_uuid = str(uuid.uuid4())
-              new_voter = Voter(uuid=voter_uuid, user = None, voter_login_id = voter['voter_id'],
-                  voter_name = voter['name'], voter_email = voter['email'], election = self.election)
-              new_voter.generate_password()
-              if election.use_voter_aliases:
-                  utils.lock_row(Election, election.id)
-                  alias_num = election.last_alias_num + 1
-                  new_voter.alias = "V%s" % alias_num
-              new_voter.save()
 
     self.processing_finished_at = datetime.datetime.utcnow()
     self.save()
diff --git a/helios/views.py b/helios/views.py
index a073015..9be3a31 100644
--- a/helios/views.py
+++ b/helios/views.py
@@ -1298,13 +1298,11 @@ def voters_upload(request, election):
         # import the first few lines to check
         try:
           voters = [v for v in voter_file_obj.itervoters()][:5]
-        except:
+          if len(voters) == 0:
+            raise Exception("no valid lines found in voter file")
+        except Exception as e:
           voters = []
-          problems.append("your CSV file could not be processed. Please check that it is a proper CSV file.")
-
-        # check if voter emails look like emails
-        if False in [v['voter_type'] in AUTH_SYSTEMS for v in voters]:
-          problems.append("those don't look like valid auth methods. Are you sure you uploaded a file with voter type as first field?")
+          problems.append("your CSV file could not be processed because %s" % str(e))
 
         return render_template(request, 'voters_upload_confirm', {'election': election, 'voters': voters, 'problems': problems})
       else:
-- 
GitLab