From 128eb6a7d46f06c4bb2a0079134a20cf5d687b43 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 26 Aug 2021 21:06:16 +0200 Subject: [PATCH] user import: Clarify fields validation --- locales/en.json | 2 +- src/yunohost/tests/test_user-group.py | 4 +-- src/yunohost/user.py | 43 +++++++++++++-------------- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/locales/en.json b/locales/en.json index 5b93dbb9f..06d2df0a4 100644 --- a/locales/en.json +++ b/locales/en.json @@ -634,7 +634,7 @@ "user_updated": "User info changed", "user_import_bad_line": "Incorrect line {line}: {details}", "user_import_bad_file": "Your CSV file is not correctly formatted it will be ignored to avoid potential data loss", - "user_import_missing_column": "The column {column} is missing", + "user_import_missing_columns": "The following columns are missing: {columns}", "user_import_partial_failed": "The users import operation partially failed", "user_import_failed": "The users import operation completely failed", "user_import_nothing_to_do": "No user needs to be imported", diff --git a/src/yunohost/tests/test_user-group.py b/src/yunohost/tests/test_user-group.py index 344a20fed..bbedfc27f 100644 --- a/src/yunohost/tests/test_user-group.py +++ b/src/yunohost/tests/test_user-group.py @@ -10,7 +10,7 @@ from yunohost.user import ( user_update, user_import, user_export, - CSV_FIELDNAMES, + FIELDS_FOR_IMPORT, FIRST_ALIASES, user_group_list, user_group_create, @@ -151,7 +151,7 @@ def test_import_user(mocker): user_import(csv_io, update=True, delete=True) group_res = user_group_list()['groups'] - user_res = user_list(CSV_FIELDNAMES)['users'] + user_res = user_list(list(FIELDS_FOR_IMPORT.keys()))['users'] assert "albert" in user_res assert "alice" in user_res assert "bob" not in user_res diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 076d930ca..b055d2cdb 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -43,8 +43,7 @@ from yunohost.log import is_unit_operation logger = getActionLogger("yunohost.user") -CSV_FIELDNAMES = [u'username', u'firstname', u'lastname', u'password', u'mailbox-quota', u'mail', u'mail-alias', u'mail-forward', u'groups'] -VALIDATORS = { +FIELDS_FOR_IMPORT = { 'username': r'^[a-z0-9_]+$', 'firstname': r'^([^\W\d_]{1,30}[ ,.\'-]{0,3})+$', 'lastname': r'^([^\W\d_]{1,30}[ ,.\'-]{0,3})+$', @@ -619,10 +618,10 @@ def user_export(): import csv # CSV are needed only in this function from io import StringIO with StringIO() as csv_io: - writer = csv.DictWriter(csv_io, CSV_FIELDNAMES, + writer = csv.DictWriter(csv_io, list(FIELDS_FOR_IMPORT.keys()), delimiter=';', quotechar='"') writer.writeheader() - users = user_list(CSV_FIELDNAMES)['users'] + users = user_list(list(FIELDS_FOR_IMPORT.keys()))['users'] for username, user in users.items(): user['mail-alias'] = ','.join(user['mail-alias']) user['mail-forward'] = ','.join(user['mail-forward']) @@ -672,24 +671,24 @@ def user_import(operation_logger, csvfile, update=False, delete=False): users_in_csv = [] existing_users = user_list()['users'] - past_lines = [] reader = csv.DictReader(csvfile, delimiter=';', quotechar='"') - for user in reader: - # Validation - try: - format_errors = [key + ':' + str(user[key]) - for key, validator in VALIDATORS.items() - if user[key] is None or not re.match(validator, user[key])] - except KeyError as e: - logger.error(m18n.n('user_import_missing_column', - column=str(e))) - is_well_formatted = False - break - if 'username' in user: - if user['username'] in past_lines: - format_errors.append('username: %s (duplicated)' % user['username']) - past_lines.append(user['username']) + missing_columns = [key for key in FIELDS_FOR_IMPORT.keys() if key not in reader.fieldnames] + if missing_columns: + raise YunohostValidationError("user_import_missing_columns", columns=', '.join(missing_columns)) + + for user in reader: + + # Validate column values against regexes + format_errors = [key + ':' + str(user[key]) + for key, validator in FIELDS_FOR_IMPORT.items() + if user[key] is None or not re.match(validator, user[key])] + + # Check for duplicated username lines + if user['username'] in users_in_csv: + format_errors.append(f'username: {user[username]} (duplicated)') + users_in_csv.append(user['username']) + if format_errors: logger.error(m18n.n('user_import_bad_line', line=reader.line_num, @@ -714,8 +713,6 @@ def user_import(operation_logger, csvfile, update=False, delete=False): elif update: actions['updated'].append(user) - users_in_csv.add(user['username']) - if delete: actions['deleted'] = [user for user in existing_users if user not in users_in_csv] @@ -787,7 +784,7 @@ def user_import(operation_logger, csvfile, update=False, delete=False): for group in new_infos['groups']: user_group_update(group, add=new_infos['username'], sync_perm=False, from_import=True) - users = user_list(CSV_FIELDNAMES)['users'] + users = user_list(list(FIELDS_FOR_IMPORT.keys()))['users'] operation_logger.start() # We do delete and update before to avoid mail uniqueness issues for user in actions['deleted']: