Fixes #6492 - ipmi_boot permission renamed to ipmi_boot_hosts #3926
Conversation
| assert_redirected_to host_path(@host.id) | ||
| end | ||
|
|
||
| test 'responds gracefully if BMC is not available' do |
There was a problem hiding this comment.
This test name should be changed, perhaps to say it responds correctly for a non-admin user or similar.
| @@ -295,7 +295,7 @@ | |||
| created_at: "2013-12-04 08:41:04.931564" | |||
| updated_at: "2013-12-04 08:41:04.931564" | |||
| ipmi_boot: | |||
There was a problem hiding this comment.
Nitpick, update the name here too, please.
| class RenameIpmiBootPermission < ActiveRecord::Migration | ||
| def change | ||
| Permission.where(:name => 'ipmi_boot'). | ||
| update_all(:name => 'ipmi_boot_hosts') |
There was a problem hiding this comment.
This change isn't reversible, it'll need separate up/down methods.
| @@ -305,6 +305,8 @@ def nics | |||
| end | |||
|
|
|||
| def ipmi_boot | |||
| raise Foreman::Exception.new( | |||
There was a problem hiding this comment.
This might need to be in the API v2 hosts controller (or the model) too? it calls ipmi_boot without any checking.
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
Authorizer expects permission names to follow a convention 'action'_'controller'. However this permission was not following it, and this prevented the permission from being applied properly. Before this fix, only admins could call ipmi_boot. I've also added a small fix to the controller to check whether the BMC interface is available before making the IPMI call - otherwise the error that Foreman threw did not make much sense for the end user (NoMethodError on bmc_proxy).
|
Thanks for taking a look @domcleal - I've updated it, the changes include:
|
| @@ -351,7 +353,7 @@ def detect_host_type | |||
| end | |||
|
|
|||
| def permissions_check | |||
| permission = "#{params[:action]}_hosts".to_sym | |||
| permission = "#{action_permission}_hosts".to_sym | |||
There was a problem hiding this comment.
Without this, we call for permissions that might not even exist in the next line. E.g: 'rebuild_config_hosts', 'boot_hosts', etc... I think this fixes permission problems with a bunch of other methods, not just 'ipmi_boot_hosts'
|
Merged, thanks @dLobatog. |
Authorizer expects permission names to follow a convention
'action'_'controller'. However this permission was not following it, and
this prevented the permission from being applied properly.
Before this fix, only admins could call ipmi_boot. I've also added a
small fix to the controller to check whether the BMC interface is
available before making the IPMI call - otherwise the error that Foreman
threw did not make much sense for the end user (NoMethodError on
bmc_proxy).