theforeman / foreman_expire_hosts

Foreman plugin for limiting host lifetime
GNU General Public License v3.0
5 stars 12 forks source link

extend built in permissions #41

Closed timogoebel closed 5 years ago

laugmanuel commented 5 years ago

This patch seems to introduce or cause two new bugs:

  1. If the user account has permission to read/list hosts but not edit them, the "Change expiration" bulk action results in a popup listing all systems (seems like the default "Hosts" list).
  2. Even if the user has sufficient permissions to change a host, a error is thrown. However, the expiration date will be successfully set for the selected systems.:
    
    wrong number of arguments (given 1, expected 0)
    /opt/theforeman/tfm-ror52/root/usr/share/gems/gems/actionpack-5.2.1/lib/action_controller/metal/flash.rb:36:in `block (2 levels) in add_flash_types'
    /opt/theforeman/tfm/root/usr/share/gems/gems/foreman_expire_hosts-5.1.0/app/controllers/concerns/foreman_expire_hosts/host_controller_extensions.rb:36:in `update_multiple_expiration'
    /opt/theforeman/tfm-ror52/root/usr/share/gems/gems/actionpack-5.2.1/lib/action_controller/metal/basic_implicit_render.rb:6:in `send_action'
    /opt/theforeman/tfm-ror52/root/usr/share/gems/gems/actionpack-5.2.1/lib/abstract_controller/base.rb:194:in `process_action'
    [...]```
timogoebel commented 5 years ago

@laugmanuel: These issues should be resolved in the 6.0 branch of this plugin.

laugmanuel commented 5 years ago

@timogoebel thanks for the advice. I did upgrade the plugin to version 6.0.1. Unfortunately it did only resolve the second problem (causing the stack trace). There is still the weird behaviour when I try to change a host where I don't have enough permissions. This might be a result of the different roles we use for different access levels (read vs. edit).

timogoebel commented 5 years ago

I'll go ahead and merge this one, we can take a look at the other issue and open a separate PR with a fix.