-
Notifications
You must be signed in to change notification settings - Fork 3.3k
VM/VMSS: incorporate credentials validation logic used by portal #2537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -523,15 +523,7 @@ def _validate_vm_create_auth(namespace): | |
| StorageProfile.SASpecializedOSDisk]: | ||
| return | ||
|
|
||
| if len(namespace.admin_username) < 6 or namespace.admin_username.lower() == 'root': | ||
| # prompt for admin username if inadequate | ||
| from azure.cli.core.prompting import prompt, NoTTYException | ||
| try: | ||
| logger.warning("Cannot use admin username: %s. Admin username should be at " | ||
| "least 6 characters and cannot be 'root'", namespace.admin_username) | ||
| namespace.admin_username = prompt('Admin Username: ') | ||
| except NoTTYException: | ||
| raise CLIError('Please specify a valid admin username in non-interactive mode.') | ||
| _validate_admin_username(namespace.admin_username, namespace.os_type.lower() == 'linux') | ||
|
|
||
| if not namespace.os_type: | ||
| raise CLIError("Unable to resolve OS type. Specify '--os-type' argument.") | ||
|
|
@@ -551,13 +543,16 @@ def _validate_vm_create_auth(namespace): | |
| "incorrect usage for authentication-type 'password': " | ||
| "[--admin-username USERNAME] --admin-password PASSWORD") | ||
|
|
||
| if not namespace.admin_password: | ||
| # prompt for admin password if not supplied | ||
| from azure.cli.core.prompting import prompt_pass, NoTTYException | ||
| try: | ||
| from azure.cli.core.prompting import prompt_pass, NoTTYException | ||
| try: | ||
| if not namespace.admin_password: | ||
| namespace.admin_password = prompt_pass('Admin Password: ', confirm=True) | ||
| except NoTTYException: | ||
| raise CLIError('Please specify both username and password in non-interactive mode.') | ||
| except NoTTYException: | ||
| raise CLIError('Please specify password in non-interactive mode.') | ||
|
|
||
| # validate password | ||
| _validate_admin_password(namespace.admin_password, | ||
| namespace.os_type.lower() == 'linux') | ||
|
|
||
| elif namespace.authentication_type == 'ssh': | ||
|
|
||
|
|
@@ -571,6 +566,52 @@ def _validate_vm_create_auth(namespace): | |
| '/home/{}/.ssh/authorized_keys'.format(namespace.admin_username) | ||
|
|
||
|
|
||
| def _validate_admin_username(username, is_linux): | ||
| if not username: | ||
| return "admin user name can not be empty" | ||
|
||
| # pylint: disable=line-too-long | ||
| pattern = (r'[\\\/"\[\]:|<>+=;,?*@#()!A-Z]+' if is_linux else r'[\\\/"\[\]:|<>+=;,?*@]+') | ||
| linux_err = r'admin user name cannot contain upper case character A-Z, special characters \/"[]:|<>+=;,?*@#()! or start with $ or -' | ||
| win_err = r'admin user name cannot contain special characters \/"[]:|<>+=;,?*@# or ends with .' | ||
| if re.findall(pattern, username): | ||
| raise CLIError(linux_err if is_linux else win_err) | ||
| if is_linux and re.findall(r'^[$-]+', username): | ||
| raise CLIError(linux_err) | ||
| if not is_linux and username.endswith('.'): | ||
| raise CLIError(win_err) | ||
| disallowed_user_names = [ | ||
| "administrator", "admin", "user", "user1", "test", "user2", | ||
| "test1", "user3", "admin1", "1", "123", "a", "actuser", "adm", | ||
| "admin2", "aspnet", "backup", "console", "david", "guest", "john", | ||
| "owner", "root", "server", "sql", "support", "support_388945a0", | ||
| "sys", "test2", "test3", "user4", "user5"] | ||
| if username.lower() in disallowed_user_names: | ||
| raise CLIError('The specified admin user name is not allowed, as it uses reserved words. Try again with a different value') | ||
|
||
|
|
||
|
|
||
| def _validate_admin_password(password, is_linux): | ||
| max_length = 72 if is_linux else 123 | ||
| min_length = 12 | ||
| if len(password) not in range(min_length, max_length + 1): | ||
| raise CLIError('The pssword length must be between {} and {}'.format(min_length, | ||
| max_length)) | ||
| contains_lower = re.findall('[a-z]+', password) | ||
| contains_upper = re.findall('[A-Z]+', password) | ||
| contains_digit = re.findall('[0-9]+', password) | ||
| contains_special_char = re.findall(r'[ `~!@#$%^&*()=+_\[\]{}\|;:.\/\'\",<>?]+', password) | ||
| count = len([x for x in [contains_lower, contains_upper, | ||
| contains_digit, contains_special_char] if x]) | ||
| # pylint: disable=line-too-long | ||
| if count < 3: | ||
| raise CLIError('Password must have the 3 of the following: 1 lower case character, 1 upper case character, 1 number and 1 special character') | ||
| disallowed_passwords = [ | ||
| "abc@123", "P@$$w0rd", "P@ssw0rd", "P@ssword123", "Pa$$word", | ||
| "pass@word1", "Password!", "Password1", "Password22", "iloveyou!" | ||
| ] | ||
| if password.lower() in disallowed_passwords: | ||
| raise CLIError('The specified password is not allowed') | ||
|
||
|
|
||
|
|
||
| def validate_ssh_key(namespace): | ||
| string_or_file = (namespace.ssh_key_value or | ||
| os.path.join(os.path.expanduser('~'), '.ssh/id_rsa.pub')) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,9 @@ | |
|
|
||
| from azure.cli.command_modules.vm._validators import (validate_ssh_key, | ||
| _is_valid_ssh_rsa_public_key, | ||
| _figure_out_storage_source) | ||
| _figure_out_storage_source, | ||
| _validate_admin_username, | ||
| _validate_admin_password) | ||
|
|
||
|
|
||
| class TestActions(unittest.TestCase): | ||
|
|
@@ -65,3 +67,64 @@ def test_figure_out_storage_source(self): | |
| self.assertFalse(src_disk) | ||
| self.assertFalse(src_snapshot) | ||
| self.assertEqual(src_blob_uri, test_data) | ||
|
|
||
| def test_validate_admin_username_linux(self): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 for unit tests! |
||
| # pylint: disable=line-too-long | ||
| err_invalid_char = r'admin user name cannot contain upper case character A-Z, special characters \/"[]:|<>+=;,?*@#()! or start with $ or -' | ||
|
|
||
| self._verify_username_with_ex('david', True, 'The specified admin user name is not allowed, as it uses reserved words. Try again with a different value') | ||
| self._verify_username_with_ex('!@#', True, err_invalid_char) | ||
| self._verify_username_with_ex('dav[', True, err_invalid_char) | ||
| self._verify_username_with_ex('Adavid', True, err_invalid_char) | ||
| self._verify_username_with_ex('-ddavid', True, err_invalid_char) | ||
|
|
||
| _validate_admin_username('d-avid1', True) | ||
| _validate_admin_username('david1', True) | ||
| _validate_admin_username('david1.', True) | ||
|
|
||
| def test_validate_admin_username_windows(self): | ||
| # pylint: disable=line-too-long | ||
| err_invalid_char = r'admin user name cannot contain special characters \/"[]:|<>+=;,?*@# or ends with .' | ||
|
|
||
| self._verify_username_with_ex('david', False, 'The specified admin user name is not allowed, as it uses reserved words. Try again with a different value') | ||
| self._verify_username_with_ex('!@#', False, err_invalid_char) | ||
| self._verify_username_with_ex('dav[', False, err_invalid_char) | ||
| self._verify_username_with_ex('dddivid.', False, err_invalid_char) | ||
|
|
||
| _validate_admin_username('ADAVID', False) | ||
| _validate_admin_username('d-avid1', False) | ||
| _validate_admin_username('david1', False) | ||
|
|
||
| def test_validate_admin_password_linux(self): | ||
| # pylint: disable=line-too-long | ||
| err_length = 'The pssword length must be between 12 and 72' | ||
| err_variety = 'Password must have the 3 of the following: 1 lower case character, 1 upper case character, 1 number and 1 special character' | ||
|
|
||
| self._verify_password_with_ex('te', True, err_length) | ||
| self._verify_password_with_ex('P12' + '3' * 70, True, err_length) | ||
| self._verify_password_with_ex('te12312312321', True, err_variety) | ||
|
|
||
| _validate_admin_password('Password22345', True) | ||
| _validate_admin_password('Password12!@#', True) | ||
|
|
||
| def test_validate_admin_password_windows(self): | ||
| # pylint: disable=line-too-long | ||
| err_length = 'The pssword length must be between 12 and 123' | ||
| err_variety = 'Password must have the 3 of the following: 1 lower case character, 1 upper case character, 1 number and 1 special character' | ||
|
|
||
| self._verify_password_with_ex('P1', False, err_length) | ||
| self._verify_password_with_ex('te14' + '3' * 120, False, err_length) | ||
| self._verify_password_with_ex('te12345678997', False, err_variety) | ||
|
|
||
| _validate_admin_password('Password22!!!', False) | ||
| _validate_admin_password('Pas' + '1' * 70, False) | ||
|
|
||
| def _verify_username_with_ex(self, admin_username, is_linux, expected_err): | ||
| with self.assertRaises(CLIError) as context: | ||
| _validate_admin_username(admin_username, is_linux) | ||
| self.assertTrue(expected_err in str(context.exception)) | ||
|
|
||
| def _verify_password_with_ex(self, admin_password, is_linux, expected_err): | ||
| with self.assertRaises(CLIError) as context: | ||
| _validate_admin_password(admin_password, is_linux) | ||
| self.assertTrue(expected_err in str(context.exception)) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of taking an "is_linux" bool, just accept the os_type and do the check in this method. Otherwise you run the risk of introducing bugs in the future if one invocation is changed and the other is not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done