chef-boneyard/sudo

sudo resource ignores group string if user string set

bby-bishopclark opened this issue · 2 comments

Cookbook version

3.5.3

Chef-client version

13.7.16

Platform Details

enterprise linux 7

Scenario:

https://github.com/chef-cookbooks/sudo/blob/master/providers/default.rb#L80 ignores group setting if user is set.

sudo 'app_admins'  do
  user    'bob,dave'
  group 'web_admins,%db_admins'  # strings with a % between to fake out same Line80
  commands  ['/sbin/service httpd restart']
end

-> /etc/sudoers.d/app_admins

bob,dave ALL(ALL)=ALL /sbin/service httpd restart

Steps to Reproduce:

as in scenario

Expected Result:

bob,dave,%web_admins,%db_admins ALL(ALL)=ALL /sbin/service httpd restart

(with groups and users shown)

Actual Result:

as in scenario - groups ignored on L80

It doesn't appear to accept

user        [*v['user']].reject{|i| i==''} + [*v['group']].reject{|i| i==''}.map{|f| "%#{f}"}

either -- it'll blow chunks when user=[] or '' . My ability to fake this cookbook out is ever more impaired!

Can we consider swapping L80 with something like

sudoer = ([*new_resource.user] + [*new_resource.group].map{|grp| "%#{grp}"}).join(',')

? Will that work to get us the bare 'string xor string' logic, while still providing for users and groups in any number as string or array-of-string ?

yeah, that works.

It gives us ugly long strings, but when our preference is chef managing ONE sudoers file on a given machine to avoid vestigial cruft (show my noob self how to survey and reap that vestigial cruft?) this code still allows the classic use but also safely allows overloading.

Huh. What about the case on L80 when sudoer='' or nil? 🤔

tas50 commented

I believe I fixed this with 0375f15

We properly prepend % to the groups and accept both a comma separated list or an array. We then just combine the arrays before templating it all out. There's a test, but let me know if what I put together makes sense to you.