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

Disable System Accounts #54

Merged
merged 4 commits into from
Jun 5, 2015
Merged

Disable System Accounts #54

merged 4 commits into from
Jun 5, 2015

Conversation

igoraj
Copy link
Contributor

@igoraj igoraj commented May 18, 2015

As per CIS-CAT also system accounts should be hardened, their shell set to /usr/sbin/nologin and their password locked, so I made a few changes to support that in this module.

Could not find any section on how to contribute so let me know if this is acceptable or not or would you want something approached in different way.

@arlimus
Copy link
Member

arlimus commented May 18, 2015

@igoraj Awesome, thank you for contributing! I'll take an extra round testing this one, before merging.

@igoraj
Copy link
Contributor Author

igoraj commented May 19, 2015

@arlimus cool, and as i said, let me know if i should redo some parts.

@arlimus
Copy link
Member

arlimus commented May 24, 2015

@igoraj Sorry for the delay, I finally had a go at everything :)

I was thinking about the naming of the attribute: What do you think about skip_users or ignore_users instead of leave_alone_users? (maybe make it a bit shorter)
Other than that, I'd love to get this merged 👍

Side-note: Short-term I believe we will be able to replace the parsing of login.defs, as we also write this file. There is one more issue (added as #57) to fix to wrap it up.

@igoraj
Copy link
Contributor Author

igoraj commented May 25, 2015

ignore_users make so much more sense! there you go!
i also added validation in case class gets something else than array from user.
might be worth adding those for all input vars (but I'll do a separate PR)

@arlimus
Copy link
Member

arlimus commented May 25, 2015

@igoraj This looks great, thank you so much! :)

Funny issue, it's currently breaking my test env (not visible here yet; our test kitchen runs separately), because all vagrant test users fall inside sys-uid range (incorrectly, imho). I'll takle that env issue and finally merge this.

@igoraj
Copy link
Contributor Author

igoraj commented Jun 3, 2015

@arlimus do you have any ETA for that? can i help somehow?

@arlimus
Copy link
Member

arlimus commented Jun 5, 2015

@igoraj Sorry for the delay, i'll finally wrap this up today.

arlimus added a commit that referenced this pull request Jun 5, 2015
Disable System Accounts
@arlimus arlimus merged commit b444569 into dev-sec:master Jun 5, 2015
@arlimus
Copy link
Member

arlimus commented Jun 5, 2015

I'll bump the release at the end of next week. Thank you for adding this :)

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.

2 participants