remove authorized_keys file if no keys exist, also cleanup authorized_keys2 files

[CpuID]

authorized_keys2 was phased out in 2001, but on existing systems may be populated. this makes sure both files are chef managed, and either dont exist or exist in the state we expect. best result security wise for the pubkey files.

[chef-supermarket]

Hi. I am an automated pull request bot named Curry. There are commits in this pull request whose authors are not yet authorized to contribute to Chef Software, Inc. projects or are using a non-GitHub verified email address. To become authorized to contribute, you will need to sign the Contributor License Agreement (CLA) as an individual or on behalf of your company. You can read more on Chef's blog.

GitHub Users Who Are Not Authorized To Contribute

The following GitHub users do not appear to have signed a CLA:

• @CpuID

Please sign the CLA here.

[chef-supermarket]

Hi. Your friendly Curry bot here. Just letting you know that all commit authors have become authorized to contribute. I have added the "Signed CLA" label to this issue so it can easily be found in the future.

[nkadel-skyhook]

Please, no. This will overwrite users's own locally managed authorized_keys who've not opted into chef management of their individual accounts.

If you want a blank authorized_keys, publish a "#comment" key as below.

{ "id": "username", "password": "!!", "groups": [ "username" ], "ssh_keys": "#No key currently enabled", "uid": 10000, "shell": "\/sbin\/nologin", "action": "create", "comment": "Username test user" }

[asciifaceman]

They behavior is not desired. Nor is it idempotent. You either manage a file via chef or you don't, there can't be a hybrid On Oct 6, 2015 1:13 PM, "Nico Kadel-Garcia" notifications@github.com wrote:

Please, no. This will overwrite users's own locally managed authorized_keys who've not opted into chef management of their individual accounts.

If you want a blank authorized_keys, publish a "#comment" key as below.

{ "id": "username", "password": "!!", "groups": [ "username" ], "ssh_keys": "#No key currently enabled", "uid": 10000, "shell": "\/sbin\/nologin", "action": "create", "comment": "Username test user" }

— Reply to this email directly or view it on GitHub https://github.com/chef-cookbooks/users/pull/90#issuecomment-145984717.

[nkadel-skyhook]

I've myself used the 'users' cookbook on a designated master server to enable default accounts for NFS access and uid consistency, without trying to take on the task of maintaining individual user's private or publish SSH keys. So the use case exists.

I also suggest that setting the credentials to be ‘#No key currently enabled’ would, indeed, be idempotent. Run twice, it would generate precisely the same result of disabling unspecified public key access, which is what seems to concern you.

Unfortunately, there are multiple public and private cookbooks which may themselves generate individual authorized_keys, such as the “rsnapshot” cookbook. And because the ‘users’ cookbook relies on data bags, these configuratons are not easily modified for specific instances. The result of your change would be to lock out SSH key access for every chef managed user account, whether configured individually or by non-Chef tools,

So rather than breaking other configuration tools or individual private access, it seems much safer to leave alone the existing ‘do not touch authorized_keys unless asked to’ behavior. Even if the default behavior changes, the "do not mess with SSH keys" should at least be retained as an option, and I’m afraid your current code doesn’t permit this.

[iennae]

@CpuID thanks for the contribution! Upon review, as implemented this has some danger zone areas as @nkadel-skyhook has pointed out. Since this is a community cookbook ( and I have just recently experienced my own assumptions failing to cover edge cases about this cookbook), I think that adding this change in behavior to delete a file that was not originally managed by Chef as somewhat dangerous. Do you think you could rewrite this to be managed by a resource parameter instead so that folks actively could choose this behavior as desired?

Thanks!

[iennae]

Closing as no response. If you do want to continue with this, please take a look at my previous comment, and rewrite to be managed by a resource parameter. Thanks!