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

Add attribute to control login.defs PASS_WARN_AGE #135

Merged
merged 2 commits into from
Feb 3, 2017

Conversation

ncs-alane
Copy link
Contributor

@ncs-alane ncs-alane commented Jan 24, 2017

Adds node['os-hardening']['auth']['pass_warn_age'] with a default of 7, and passes it to the /etc/login.defs template.

@coveralls
Copy link

coveralls commented Jan 24, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling de23d71 on ncs-alane:ncs-alane-pass-warn-age into 720c6bc on dev-sec:master.

@coveralls
Copy link

coveralls commented Jan 25, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 8595be4 on ncs-alane:ncs-alane-pass-warn-age into 720c6bc on dev-sec:master.

@coveralls
Copy link

coveralls commented Jan 25, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling f0d9d2b on ncs-alane:ncs-alane-pass-warn-age into 720c6bc on dev-sec:master.

@atomic111
Copy link
Member

@ncs-alane thanks for the nice pr. i need some time to review it. can you please add a short description about the new attribute in the README.md

@ncs-alane
Copy link
Contributor Author

@atomic111 sure thing!

@coveralls
Copy link

coveralls commented Jan 25, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling b71ef40 on ncs-alane:ncs-alane-pass-warn-age into 720c6bc on dev-sec:master.

Copy link
Member

@artem-sidorenko artem-sidorenko left a comment

Choose a reason for hiding this comment

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

@ncs-alane thank you for this PR!

Could you please move the test improvements (cached chef-runs and changes of expect/syntax) to a dedicated PR (see my question to @chris-rock above regarding the syntax change). Its a bit hard to review, still if there is a good commit order. The main intention of this PR is to introduce the pass_warn_age and not to improve the tests, still if this is a good thing :-)

expect(chef_run).to include_recipe 'os-hardening::profile'
expect(chef_run).to include_recipe 'os-hardening::securetty'
expect(chef_run).to include_recipe 'os-hardening::sysctl'
is_expected.to include_recipe 'os-hardening::packages'
Copy link
Member

@artem-sidorenko artem-sidorenko Jan 26, 2017

Choose a reason for hiding this comment

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

@chris-rock as far I can remember we have everywhere the expect(chef_run) syntax. What do you think, is this change Ok for us?

I personally would like to have this changes everywhere (incl. other chef repos) and in the dedicated PR(s).

@ncs-alane
Copy link
Contributor Author

@artem-sidorenko okay, I've reverted the test changes and moved them to #136.

@coveralls
Copy link

coveralls commented Jan 26, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 6f22c61 on ncs-alane:ncs-alane-pass-warn-age into 720c6bc on dev-sec:master.

@artem-sidorenko
Copy link
Member

@ncs-alane that looks much more better :-) Many thanks for splitting of this PR. May I ask you to cleanup the commit history by squashing the commits?

@atomic111 LGTM when the history is clean

@ncs-alane ncs-alane force-pushed the ncs-alane-pass-warn-age branch from 6f22c61 to b6d70c9 Compare January 31, 2017 05:09
@coveralls
Copy link

coveralls commented Jan 31, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling b6d70c9 on ncs-alane:ncs-alane-pass-warn-age into 720c6bc on dev-sec:master.

@ncs-alane
Copy link
Contributor Author

@artem-sidorenko done like dinner!

@artem-sidorenko
Copy link
Member

@ncs-alane thank you! Lets wait a bit for @atomic111 to have look (it looks like he is a bit busy currently with something). I'll try to review the test improvements in the next days

@atomic111
Copy link
Member

@ncs-alane thank you for the great work!!!

@atomic111 atomic111 merged commit 19003eb into dev-sec:master Feb 3, 2017
@ncs-alane
Copy link
Contributor Author

@atomic111 you're very welcome!

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.

5 participants