rabbitmq / chef-cookbook

Development repository for Chef cookbook RabbitMQ
https://supermarket.chef.io/cookbooks/rabbitmq
Apache License 2.0
214 stars 423 forks source link

Fix :clear_permissions action in user provider #530

Closed TheFLHurricane closed 5 years ago

TheFLHurricane commented 5 years ago

Proposed Changes

529 :clear_permissions action for user provider is not executed in some cases when it should

:clear_permissions fails to ever trigger in user provider because a user with permissions can never have a true result against a nil perm_list that is passed to the user_has_permissions? method, which is necessary to trigger the action. Fixing this by changing check method to check if correct permissions exist and not just any permissions.
Specific details:

  1. Change result of first return (perm_list.nil? && cmd.stdout.empty?) from false to true
  2. Change action :clear_permissions to call if method instead of unless method
  3. Change name of method from user_has_permissions? to user_has_correct_permissions? to accurately reflect result being returned

Types of Changes

What types of changes does your code introduce to this project? Put an x in the boxes that apply

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask on the mailing list. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

Further Comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc.

michaelklishin commented 5 years ago

Thanks. We need to add some tests. I've submitted #531 with minor modifications, let's continue there.