Skip to content
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

FM-1900 Add User defined type #68

Merged
merged 1 commit into from Feb 4, 2015
Merged

FM-1900 Add User defined type #68

merged 1 commit into from Feb 4, 2015

Conversation

cyberious
Copy link
Contributor

No description provided.

validate_re($password, '^.{8,128}$', 'Password must be bewteen 8 and 128 characters')
validate_re($password,'[A-Z]{1,}','Password must contain at least one uppercase letter')
validate_re($password,'[a-z]{1,}','Password must contain at least one lowercase letter')
validate_re($password,'\W+','Password must contain at least one special character')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused about why we're enforcing password requirements here. Any rules in place on the SQL Server side should bounce out an error message about unmet requirements.

Also, FYI - password length is a stronger indicator of security than inclusion of upper / lower / special characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to ensure we fail fast and we know these requirements so if we can do thing ahead of spinning up powershell twice it would be preferential.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand - but given the user can configure the password policy, I don't think this is appropriate here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can understand the special character and maybe the one uppercase and lowercase but length I believe is a rule across the board, especially on max length.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those errors will bubble up to the user so I am fine, just let me know if you are vieamently apposed to the validation. I am sure most if not all DBA's would at least want the top three met. A number is another one we didn't check for. Most of the time however the password will not even be used as the authentication will be done at the login level not the user level. Only in a contained database will password even be available.

@cyberious
Copy link
Contributor Author

Pinned rspec-puppet to ~> 1.0 as it broke expectations with release of 2.0

@@ -5,6 +5,7 @@ script: "bundle exec rake spec SPEC_OPTS='--format documentation'"
rvm:
- 1.9.3
- 2.0.0
- 2.1.5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ferventcoder
Copy link
Contributor

The commit message could use a bit of love.

@Iristyle
Copy link
Contributor

Iristyle commented Feb 4, 2015

👍 to more info in commit message (including ticket number in subject)

@cyberious
Copy link
Contributor Author

Updated commit message to reference ticket.

#
#
#
##
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this reserved for comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have yet to do the comments on this defined type. Docs will be there, still not done with this defined type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right on. 👍

ferventcoder added a commit that referenced this pull request Feb 4, 2015
FM-1900 Add User defined type
@ferventcoder ferventcoder merged commit 58be6dd into puppetlabs:master Feb 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants