From 463e88347811e30c123f507a03b86653fa73afab Mon Sep 17 00:00:00 2001 From: Vivek Chand Date: Sun, 14 Jul 2019 11:06:55 +0530 Subject: [PATCH] [bug] Fix failing tests in openwisp_radius #236 Closes #236 --- django_freeradius/base/forms.py | 1 + django_freeradius/base/models.py | 48 +++++++++++-------- .../tests/base/test_batch_add_users.py | 2 +- django_freeradius/tests/base/test_commands.py | 11 +++-- django_freeradius/tests/static/test_batch.csv | 2 +- .../tests/static/test_batch_new.csv | 3 ++ 6 files changed, 40 insertions(+), 27 deletions(-) create mode 100644 django_freeradius/tests/static/test_batch_new.csv diff --git a/django_freeradius/base/forms.py b/django_freeradius/base/forms.py index d7541cea..299d8849 100644 --- a/django_freeradius/base/forms.py +++ b/django_freeradius/base/forms.py @@ -65,6 +65,7 @@ def clean(self): number_of_users = data.get('number_of_users') if strategy == 'prefix' and not number_of_users: self.add_error('number_of_users', 'This field is required') + super().clean() return data def __init__(self, *args, **kwargs): diff --git a/django_freeradius/base/models.py b/django_freeradius/base/models.py index 6285bb31..a008c2f5 100644 --- a/django_freeradius/base/models.py +++ b/django_freeradius/base/models.py @@ -26,6 +26,8 @@ from .. import settings as app_settings +User = get_user_model() + RADOP_CHECK_TYPES = (('=', '='), (':=', ':='), ('==', '=='), @@ -740,31 +742,14 @@ def clean(self): super(AbstractRadiusBatch, self).clean() def add(self, reader, password_length=BATCH_DEFAULT_PASSWORD_LENGTH): - User = get_user_model() users_list = [] generated_passwords = [] for row in reader: if len(row) == 5: - username, password, email, first_name, last_name = row - if not username and email: - username = email.split('@')[0] - username = find_available_username(username, users_list) - user = User(username=username, - email=email, - first_name=first_name, - last_name=last_name) - cleartext_delimiter = 'cleartext$' - if not password: - password = get_random_string(length=password_length) - user.set_password(password) - generated_passwords.append([username, password, email]) - elif password and password.startswith(cleartext_delimiter): - password = password[len(cleartext_delimiter):] - user.set_password(password) - else: - user.password = password - user.full_clean() + user, password = self.get_or_create_user(row, users_list, password_length) users_list.append(user) + if password: + generated_passwords.append(password) for user in users_list: self.save_user(user) for element in generated_passwords: @@ -796,6 +781,29 @@ def prefix_add(self, prefix, n, password_length=BATCH_DEFAULT_PASSWORD_LENGTH): self.full_clean() self.save() + def get_or_create_user(self, row, users_list, password_length): + generated_password = None + username, password, email, first_name, last_name = row + if not username and email: + username = email.split('@')[0] + username = find_available_username(username, users_list) + user = User(username=username, + email=email, + first_name=first_name, + last_name=last_name) + cleartext_delimiter = 'cleartext$' + if not password: + password = get_random_string(length=password_length) + user.set_password(password) + generated_password = ([username, password, email]) + elif password and password.startswith(cleartext_delimiter): + password = password[len(cleartext_delimiter):] + user.set_password(password) + else: + user.password = password + user.full_clean() + return user, generated_password + def save_user(self, user): user.save() self.users.add(user) diff --git a/django_freeradius/tests/base/test_batch_add_users.py b/django_freeradius/tests/base/test_batch_add_users.py index 5de5f571..db3b2ea3 100644 --- a/django_freeradius/tests/base/test_batch_add_users.py +++ b/django_freeradius/tests/base/test_batch_add_users.py @@ -32,7 +32,7 @@ def test_generate_username_when_repeat(self): hashed_password = 'pbkdf2_sha256$100000$x3DUBnOFwraV$PU2dZZq1FcuBjagxVLPhhFvpicLn18fFCN5xiLsxATc=' cleartext_password = 'cleartext$password' reader = [['rohith', cleartext_password, 'rohith@openwisp.com', 'Rohith', 'ASRK'], - ['rohith', hashed_password, 'rohith@openwisp.com', '', '']] + ['rohith', hashed_password, 'rohith@openwisp.org', '', '']] batch = self._create_radius_batch(name='test', strategy='csv', csvfile=self._get_csvfile(reader)) diff --git a/django_freeradius/tests/base/test_commands.py b/django_freeradius/tests/base/test_commands.py index 26385eb4..5a43e86a 100644 --- a/django_freeradius/tests/base/test_commands.py +++ b/django_freeradius/tests/base/test_commands.py @@ -47,16 +47,17 @@ def test_batch_add_users_command(self): radiusbatch = self.radius_batch_model.objects.first() self.assertEqual(get_user_model().objects.all().count(), 3) self.assertEqual(radiusbatch.expiration_date.strftime('%d-%m-%y'), '28-01-18') + path = self._get_path('static/test_batch_new.csv') options = dict(file=path, name='test1') self._call_command('batch_add_users', **options) self.assertEqual(self.radius_batch_model.objects.all().count(), 2) self.assertEqual(get_user_model().objects.all().count(), 6) invalid_csv_path = self._get_path('static/test_batch_invalid.csv') with self.assertRaises(CommandError): - options = dict(file='doesnotexist.csv', name='test2') + options = dict(file='doesnotexist.csv', name='test3') self._call_command('batch_add_users', **options) with self.assertRaises(SystemExit): - options = dict(file=invalid_csv_path, name='test3') + options = dict(file=invalid_csv_path, name='test4') self._call_command('batch_add_users', **options) def test_deactivate_expired_users_command(self): @@ -71,9 +72,9 @@ def test_delete_old_users_command(self): path = self._get_path('static/test_batch.csv') options = dict(file=path, expiration='28-01-1970', name='test') self._call_command('batch_add_users', **options) - expiration_date = now() - timedelta(days=30 * 15) - options['expiration'] = expiration_date.strftime('%d-%m-%Y') - options['name'] = 'test1' + expiration_date = (now() - timedelta(days=30 * 15)).strftime('%d-%m-%Y') + path = self._get_path('static/test_batch_new.csv') + options = dict(file=path, expiration=expiration_date, name='test1') self._call_command('batch_add_users', **options) self.assertEqual(get_user_model().objects.all().count(), 6) call_command('delete_old_users') diff --git a/django_freeradius/tests/static/test_batch.csv b/django_freeradius/tests/static/test_batch.csv index aee91492..c499c2a4 100644 --- a/django_freeradius/tests/static/test_batch.csv +++ b/django_freeradius/tests/static/test_batch.csv @@ -1,3 +1,3 @@ rohithasrk,cleartext$password,rohith.asrk@openwisp.com,Rohith,ASRK rohithasrk,pbkdf2_sha256$100000$x3DUBnOFwraV$PU2dZZq1FcuBjagxVLPhhFvpicLn18fFCN5xiLsxATc=,rohith@openwisp.com,, -,,rohith.asrk@openwisp.com,, +,,rohith.asrk1@openwisp.com,, diff --git a/django_freeradius/tests/static/test_batch_new.csv b/django_freeradius/tests/static/test_batch_new.csv new file mode 100644 index 00000000..c1daac0e --- /dev/null +++ b/django_freeradius/tests/static/test_batch_new.csv @@ -0,0 +1,3 @@ +vivekchand,cleartext$password,vivek.chand@openwisp.com,Vivek,Chand +vivekchand,pbkdf2_sha256$100000$x3DUBnOFwraV$PU2dZZq1FcuBjagxVLPhhFvpicLn18fFCN5xiLsxATc=,vivek@openwisp.com,, +,,vivek.rajput@openwisp.com,,