Security enhancements #142
Conversation
…validation message
|
This is great stuff man, thanks for the contribution! All the commits look very feasible. The only one that I might have some push back on is the Code Injection through the Apache commands. I completely understand the risk associated with it, but perhaps there is a secure way we can handle allowing different commands to be run for different operating systems? |
|
The other item I thought of is if we are going to lock accounts should we provide a way to unlock the account through the "user management" section of PF? |
| expect(build(:admin, password: "short")).to_not be_valid | ||
| expect(build(:admin, password: "1Short")).to_not be_valid | ||
| end | ||
|
|
There was a problem hiding this comment.
love the addition of these tests, well played!
|
Regarding the Apache commands: The only thought that comes to mind is to move global configuration to a settings file instead of database. No worries on pushing back on that commit this time around, Can take our time on solving that issue. Good call on the admin UI to unlock accounts, I've added that functionality. Thanks! |
|
Awesome, thanks for adding that UI functionality to unlock. Yeah I concur that some sort of settings.yml file to change the apache commands is a good approach. If I'm gonna be really nit picky, it looks like the tab indentations don't align with the code properly (based on what github is showing me). |
|
No worries, that was ugly. |
| validates_format_of :name, :with => /[A-Za-z\d]([-\w]{,498}[A-Za-z\d])?/i, :message => "Invalid Name: Alphanumerics only" | ||
| validates_format_of :username, :with => /[A-Za-z\d]([-\w]{,498}[A-Za-z\d])?/i, :message => "Invalid Username: Alphanumerics only" | ||
| validate :validate_password_complexity |
|
couple last indent issues and we should be good to go here! I'll probably take this branch and add in some sort of settings.yaml file to load in the apache commands. |
|
Howdy! Pushed a commit last week fixing the last indent issues (I think), Let me know if you need anything else with this code to get it on your master branch. Thanks. |
|
Appreciate for the updates here. I do plan to take advantage of these commits soon. Thanks in advance for being patient while I test and tweak. |
| @@ -107,7 +107,6 @@ | |||
| <div class="col-xs-8"> | |||
| <%= ff.password_field(:smtp_password, | |||
| placeholder: 'password', | |||
| value: @campaign.email_settings.smtp_password, | |||
There was a problem hiding this comment.
Afaik this will mean whenever you save the template settings you will wipe the password unless you re-enter it each time.
This is the result of a security audit run on PF and suggestions it has generated. This is also in preparation to possibly use in a multi-tenant environment.
Feel free to cherry-pick commits, I realize some commits take away original functionality.
Thank you for Phishing Frenzy!