-
Notifications
You must be signed in to change notification settings - Fork 60
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
Add blacklists for VM username and password when provisioning #88
Conversation
@djberg96 it seems fine to me. I've pinged @gmcculloug to take a look, too. |
'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' |
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.
I see this list is taken directly from the MS doc linked in the description but it would be easier to read if similar items were grouped together? Like all the admin
based ones and all the user#
ones.
Just curious if you have any idea why they blacklist support_388945a0
, david
and john
? They seem odd compared to something obvious like admin
and user1
.
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.
Probably best to put them in alphabetical order I suppose. I can fix that. I'm not sure about "david" or "john", other than being super common first names, but "support_388945a0" is a default built-in user account. More info if you care:
https://technet.microsoft.com/en-us/library/cc779144(WS.10).aspx
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.
Sounds good. I would say this is ready to merge once the list is ordered.
Checked commit https://github.com/djberg96/manageiq-providers-azure/commit/fa8c1c5627e8fbb5097425a8f2e677f28fb30cb4 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
👍 but sad to know my Azure password cannot be |
@djberg96 Should this PR and ManageIQ/manageiq#15513 be labeled as |
@miq-bot add_label fine/yes |
Fine backport (to manageiq repo) details:
|
In addition to certain pattern requirements, Azure has special blacklists that are considered invalid for usernames and/or passwords when provisioning a VM. These blacklists are too long to be practical to implement as part of a regex, so I've added them here as a separate list.
In addition, I've capped the username length at 20 characters to match the Azure spec.
See https://docs.microsoft.com/en-us/azure/virtual-machines/windows/faq for more details.
Requires ManageIQ/manageiq#15513 first. (Update: merged)
Addresses https://bugzilla.redhat.com/show_bug.cgi?id=1454829