diff --git a/powerdnsadmin/lib/errors.py b/powerdnsadmin/lib/errors.py index 687f554..35c32a3 100644 --- a/powerdnsadmin/lib/errors.py +++ b/powerdnsadmin/lib/errors.py @@ -129,6 +129,13 @@ class AccountNotExists(StructuredException): self.message = message self.name = name +class InvalidAccountNameException(StructuredException): + status_code = 400 + + def __init__(self, name=None, message="The account name is invalid"): + StructuredException.__init__(self) + self.message = message + self.name = name class UserCreateFail(StructuredException): status_code = 500 @@ -138,7 +145,6 @@ class UserCreateFail(StructuredException): self.message = message self.name = name - class UserCreateDuplicate(StructuredException): status_code = 409 diff --git a/powerdnsadmin/models/account.py b/powerdnsadmin/models/account.py index 4d08fc1..ab2341e 100644 --- a/powerdnsadmin/models/account.py +++ b/powerdnsadmin/models/account.py @@ -3,6 +3,7 @@ from flask import current_app from urllib.parse import urljoin from ..lib import utils +from ..lib.errors import InvalidAccountNameException from .base import db from .setting import Setting from .user import User @@ -22,7 +23,7 @@ class Account(db.Model): back_populates="accounts") def __init__(self, name=None, description=None, contact=None, mail=None): - self.name = name + self.name = Account.sanitize_name(name) if name is not None else name self.description = description self.contact = contact self.mail = mail @@ -33,9 +34,30 @@ class Account(db.Model): self.PDNS_VERSION = Setting().get('pdns_version') self.API_EXTENDED_URL = utils.pdns_api_extended_uri(self.PDNS_VERSION) - if self.name is not None: - self.name = ''.join(c for c in self.name.lower() - if c in "abcdefghijklmnopqrstuvwxyz0123456789") + + @staticmethod + def sanitize_name(name): + """ + Formats the provided name to fit into the constraint + """ + if not isinstance(name, str): + raise InvalidAccountNameException("Account name must be a string") + + allowed_characters = "abcdefghijklmnopqrstuvwxyz0123456789" + + if Setting().get('account_name_extra_chars'): + allowed_characters += "_-." + + sanitized_name = ''.join(c for c in name.lower() if c in allowed_characters) + + if len(sanitized_name) > Account.name.type.length: + current_app.logger.error("Account name {0} too long. Truncated to: {1}".format( + sanitized_name, sanitized_name[:Account.name.type.length])) + + if not sanitized_name: + raise InvalidAccountNameException("Empty string is not a valid account name") + + return sanitized_name[:Account.name.type.length] def __repr__(self): return ''.format(self.name) @@ -68,11 +90,9 @@ class Account(db.Model): """ Create a new account """ - # Sanity check - account name - if self.name == "": - return {'status': False, 'msg': 'No account name specified'} + self.name = Account.sanitize_name(self.name) - # check that account name is not already used + # Check that account name is not already used account = Account.query.filter(Account.name == self.name).first() if account: return {'status': False, 'msg': 'Account already exists'} diff --git a/powerdnsadmin/models/setting.py b/powerdnsadmin/models/setting.py index 33b8db5..08b4b89 100644 --- a/powerdnsadmin/models/setting.py +++ b/powerdnsadmin/models/setting.py @@ -190,7 +190,8 @@ class Setting(db.Model): 'otp_field_enabled': True, 'custom_css': '', 'otp_force': False, - 'max_history_records': 1000 + 'max_history_records': 1000, + 'account_name_extra_chars': False } def __init__(self, id=None, name=None, value=None): @@ -271,15 +272,15 @@ class Setting(db.Model): def get(self, setting): if setting in self.defaults: - + if setting.upper() in current_app.config: result = current_app.config[setting.upper()] else: result = self.query.filter(Setting.name == setting).first() - + if result is not None: if hasattr(result,'value'): - result = result.value + result = result.value return strtobool(result) if result in [ 'True', 'False' ] else result @@ -287,7 +288,7 @@ class Setting(db.Model): return self.defaults[setting] else: current_app.logger.error('Unknown setting queried: {0}'.format(setting)) - + def get_records_allow_to_edit(self): return list( set(self.get_forward_records_allow_to_edit() + diff --git a/powerdnsadmin/routes/admin.py b/powerdnsadmin/routes/admin.py index 5573502..fc74eaa 100644 --- a/powerdnsadmin/routes/admin.py +++ b/powerdnsadmin/routes/admin.py @@ -1260,7 +1260,9 @@ def setting_basic(): 'allow_user_create_domain', 'allow_user_remove_domain', 'allow_user_view_history', 'bg_domain_updates', 'site_name', 'session_timeout', 'warn_session_timeout', 'ttl_options', 'pdns_api_timeout', 'verify_ssl_connections', 'verify_user_email', - 'delete_sso_accounts', 'otp_field_enabled', 'custom_css', 'enable_api_rr_history', 'max_history_records', 'otp_force' + 'delete_sso_accounts', 'otp_field_enabled', 'custom_css', 'enable_api_rr_history', 'max_history_records', 'otp_force', + 'account_name_extra_chars' + ] return render_template('admin_setting_basic.html', settings=settings) diff --git a/powerdnsadmin/routes/api.py b/powerdnsadmin/routes/api.py index 4fce368..58dc317 100644 --- a/powerdnsadmin/routes/api.py +++ b/powerdnsadmin/routes/api.py @@ -23,7 +23,7 @@ from ..lib.errors import ( AccountCreateFail, AccountUpdateFail, AccountDeleteFail, AccountCreateDuplicate, AccountNotExists, UserCreateFail, UserCreateDuplicate, UserUpdateFail, UserDeleteFail, - UserUpdateFailEmail, + UserUpdateFailEmail, InvalidAccountNameException ) from ..decorators import ( api_basic_auth, api_can_create_domain, is_json, apikey_auth, @@ -863,12 +863,15 @@ def api_create_account(): contact = data['contact'] if 'contact' in data else None mail = data['mail'] if 'mail' in data else None if not name: - current_app.logger.debug("Account name missing") - abort(400) + current_app.logger.debug("Account creation failed: name missing") + raise InvalidAccountNameException(message="Account name missing") + + sanitized_name = Account.sanitize_name(name) + account_exists = Account.query.filter(Account.name == sanitized_name).all() - account_exists = [] or Account.query.filter(Account.name == name).all() if len(account_exists) > 0: - msg = "Account {} already exists".format(name) + msg = ("Requested Account {} would be translated to {}" + " which already exists").format(name, sanitized_name) current_app.logger.debug(msg) raise AccountCreateDuplicate(message=msg) @@ -906,8 +909,9 @@ def api_update_account(account_id): if not account: abort(404) - if name and name != account.name: - abort(400) + if name and Account.sanitize_name(name) != account.name: + msg = "Account name is immutable" + raise AccountUpdateFail(msg=msg) if current_user.role.name not in ['Administrator', 'Operator']: msg = "User role update accounts" diff --git a/powerdnsadmin/routes/index.py b/powerdnsadmin/routes/index.py index 316db2d..36c7916 100644 --- a/powerdnsadmin/routes/index.py +++ b/powerdnsadmin/routes/index.py @@ -325,15 +325,15 @@ def login(): # Regexp didn't match, continue to next iteration continue - account = Account() - account_id = account.get_id_by_name(account_name=group_name) + sanitized_group_name = Account.sanitize_name(group_name) + account_id = account.get_id_by_name(account_name=sanitized_group_name) if account_id: account = Account.query.get(account_id) # check if user has permissions account_users = account.get_user() current_app.logger.info('Group: {} Users: {}'.format( - group_name, + group_name, account_users)) if user.id in account_users: current_app.logger.info('User id {} is already in account {}'.format( @@ -347,13 +347,15 @@ def login(): current_app.logger.info('User {} added to Account {}'.format( user.username, account.name)) else: - account.name = group_name - account.description = group_description - account.contact = '' - account.mail = '' + account = Account( + name=sanitized_group_name, + description=group_description, + contact='', + mail='' + ) account.create_account() history = History(msg='Create account {0}'.format( - account.name), + account.name), created_by='System') history.add() @@ -403,7 +405,7 @@ def login(): if name_prop in me and desc_prop in me: accounts_name_prop = [me[name_prop]] if type(me[name_prop]) is not list else me[name_prop] accounts_desc_prop = [me[desc_prop]] if type(me[desc_prop]) is not list else me[desc_prop] - + #Run on all groups the user is in by the index num. for i in range(len(accounts_name_prop)): description = '' @@ -413,7 +415,7 @@ def login(): account_to_add.append(account) user_accounts = user.get_accounts() - + # Add accounts for account in account_to_add: if account not in user_accounts: @@ -487,13 +489,13 @@ def login(): saml_enabled=SAML_ENABLED, error='Token required') - if Setting().get('autoprovisioning') and auth_method!='LOCAL': + if Setting().get('autoprovisioning') and auth_method!='LOCAL': urn_value=Setting().get('urn_value') Entitlements=user.read_entitlements(Setting().get('autoprovisioning_attribute')) if len(Entitlements)==0 and Setting().get('purge'): user.set_role("User") user.revoke_privilege(True) - + elif len(Entitlements)!=0: if checkForPDAEntries(Entitlements, urn_value): user.updateUser(Entitlements) @@ -1088,14 +1090,10 @@ def create_group_to_account_mapping(): def handle_account(account_name, account_description=""): - clean_name = ''.join(c for c in account_name.lower() - if c in "abcdefghijklmnopqrstuvwxyz0123456789") - if len(clean_name) > Account.name.type.length: - current_app.logger.error( - "Account name {0} too long. Truncated.".format(clean_name)) + clean_name = Account.sanitize_name(account_name) account = Account.query.filter_by(name=clean_name).first() if not account: - account = Account(name=clean_name.lower(), + account = Account(name=clean_name, description=account_description, contact='', mail='') diff --git a/powerdnsadmin/templates/admin_edit_account.html b/powerdnsadmin/templates/admin_edit_account.html index 41f2d43..1946bc9 100644 --- a/powerdnsadmin/templates/admin_edit_account.html +++ b/powerdnsadmin/templates/admin_edit_account.html @@ -49,7 +49,7 @@ {% if invalid_accountname %} Cannot be blank and must only contain alphanumeric - characters. + characters{% if SETTING.get('account_name_extra_chars') %}, dots, hyphens or underscores{% endif %}. {% elif duplicate_accountname %} Account name already in use. {% endif %} @@ -112,8 +112,9 @@

Fill in all the fields to the in the form to the left.

- Name is an account identifier. It will be stored as all lowercase letters (no - spaces, special characters etc).
+ Name is an account identifier. It will be lowercased and can contain alphanumeric + characters{% if SETTING.get('account_name_extra_chars') %}, dots, hyphens and underscores (no space or other special character is allowed) + {% else %} (no extra character is allowed){% endif %}.
Description is a user friendly name for this account.
Contact person is the name of a contact person at the account.
Mail Address is an e-mail address for the contact person.