[PATCH/foreman 1/2] Fix class to input parameter naming

Olivier_Favre · May 21, 2012, 2:43pm

Patch from ohadlevy

Signed-off-by: Olivier Favre <olivier@yakaz.com>

···

--- app/views/common/_edit_habtm.erb | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/views/common/_edit_habtm.erb b/app/views/common/_edit_habtm.erb
index 288df9e…74aaaea 100644
— a/app/views/common/_edit_habtm.erb
+++ b/app/views/common/_edit_habtm.erb
@@ -6,11 +6,11 @@

<%= content_tag_for :label, association do %>
<%= check_box_tag(

     "#{klass.class.table_name.singularize}[#{association.class.to_s.downcase}_ids][]",

     "#{klass.class.table_name.singularize}[#{ActiveModel::Naming.singular(association)}_ids][]",
     association.id, klass.send(association.class.table_name).map(&:id).include?(association.id))
   %>
   <%= contract association %>

   <%= hidden_field_tag "#{klass.class.table_name.singularize}[#{association.class.to_s.downcase}_ids][]" %>

   <%= hidden_field_tag "#{klass.class.table_name.singularize}[#{ActiveModel::Naming.singular(association)}_ids][]" %>
 <% end -%>
 </li>

<% end -%>
–
1.7.2.5

Olivier_Favre · May 21, 2012, 2:43pm

Add some rights to give non admin users so they can perform some normal actions on the hosts they own.

Signed-off-by: Olivier Favre <olivier@yakaz.com>

···

--- .../compute_resources/vms_controller.rb | 2 +- app/controllers/compute_resources_controller.rb | 22 ++++++++++++- app/models/compute_resource.rb | 33 ++++++++++++++++++++ app/models/host.rb | 2 + app/models/user.rb | 4 ++- .../compute_resources/vms/index/_libvirt.html.erb | 2 +- .../compute_resources/vms/index/_ovirt.html.erb | 2 +- app/views/hosts/_form.html.erb | 2 +- app/views/users/_filters.html.erb | 9 +++++ .../20120514113900_add_user_compute_resources.rb | 14 ++++++++ ...115814_add_compute_resources_boolean_to_user.rb | 9 +++++ lib/foreman/access_permissions.rb | 16 +++++++++ 12 files changed, 111 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20120514113900_add_user_compute_resources.rb create mode 100644 db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb

diff --git a/app/controllers/compute_resources/vms_controller.rb b/app/controllers/compute_resources/vms_controller.rb
index 370e9ba…890bf76 100644
— a/app/controllers/compute_resources/vms_controller.rb
+++ b/app/controllers/compute_resources/vms_controller.rb
@@ -60,7 +60,7 @@ class ComputeResources::VmsController < ApplicationController
private

def find_compute_resource

@compute_resource = ComputeResource.find(params[:compute_resource_id])

@compute_resource = ComputeResource.my_compute_resources.find(params[:compute_resource_id])
end

def find_vm
diff --git a/app/controllers/compute_resources_controller.rb b/app/controllers/compute_resources_controller.rb
index e0adb31…7723e16 100644
— a/app/controllers/compute_resources_controller.rb
+++ b/app/controllers/compute_resources_controller.rb
@@ -5,7 +5,14 @@ class ComputeResourcesController < ApplicationController
before_filter :find_by_id, :only => %w{show edit update destroy} + AJAX_REQUESTS

def index

values = ComputeResource.search_for(params[:search], :order => params[:order])

begin

 my_compute_resources = User.current.admin? ? ComputeResource : ComputeResource.my_compute_resources

 values = my_compute_resources.search_for(params[:search], :order => params[:order])

rescue => e
```
 error e.to_s
```

 values = my_compute_resources.search_for ""

end
respond_to do |format|
format.html { @compute_resources = values.paginate :page => params[:page] }
format.json { render :json => values }
@@ -16,10 +23,21 @@ class ComputeResourcesController < ApplicationController
@compute_resource = ComputeResource.new
end
def show
auth = User.current.admin? ? true : ComputeResource.my_compute_resources.include?(@compute_resource)
not_found and return unless auth
respond_to do |format|
```
 format.html
```

 format.json { render :json => @compute_resource }

end
end
def create
if params[:compute_resource].present? && params[:compute_resource][:provider].present?
@compute_resource = ComputeResource.new_provider params[:compute_resource]
if @compute_resource.save

   # Add the new compute resource to the user's filters

   @compute_resource.users << User.current
   process_success
 else
   process_error

@@ -32,6 +50,8 @@ class ComputeResourcesController < ApplicationController
end

def edit

auth = User.current.admin? ? true : ComputeResource.my_compute_resources.include?(@compute_resource)
not_found and return unless auth
end

def update
diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb
index b8106b6…5912b3f 100644
— a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -8,6 +8,7 @@ class ComputeResource < ActiveRecord::Base
STI_PREFIX= “Foreman::Model”

include Authorization

has_and_belongs_to_many :users, :join_table => "user_compute_resources"
validates_format_of :name, :with => /\A(\S+)\Z/, :message => "can’t be blank or contain white spaces."
validates_uniqueness_of :name
validates_presence_of :provider, :in => PROVIDERS
@@ -16,6 +17,38 @@ class ComputeResource < ActiveRecord::Base
before_save :sanitize_url
has_many :hosts
default_scope :order => ‘LOWER(compute_resources.name)’
returns reports for hosts in the User’s filter set
scope :my_compute_resources, lambda {
user = User.current
if user.admin?
```
 conditions = { }
```
else

 conditions = sanitize_sql_for_conditions([" (compute_resources.id in (?))", user.compute_resources.map(&:id)])

```
 conditions.sub!(/\s*\(\)\s*/, "")
```

 conditions.sub!(/^(?:\(\))?\s?(?:and|or)\s*/, "")

 conditions.sub!(/\(\s*(?:or|and)\s*\(/, "((")

end
{:conditions => conditions}
}
def enforce_permissions operation
We get called again with the operation being set to create
return true if operation == “edit” and new_record?
current = User.current
if current.allowed_to?("#{operation}_compute_resources".to_sym)

 # If you can create compute resources then you can create them anywhere

```
 return true if operation == "create"
```
```
 # edit or delete
```

 if current.allowed_to?("#{operation}_compute_resources".to_sym)

   return true if ComputeResource.my_compute_resources(current).include? self

```
 end
```
end
errors.add :base, “You do not have permission to #{operation} this domain”
false
end
allows to create a specific compute class based on the provider.

def self.new_provider args
raise “must provider a provider” unless provider = args[:provider]
diff --git a/app/models/host.rb b/app/models/host.rb
index 3d2520a…37bb3e2 100644
— a/app/models/host.rb
+++ b/app/models/host.rb
@@ -79,6 +79,7 @@ class Host < Puppet::Rails::Host
user = User.current
owner_conditions = sanitize_sql_for_conditions(["((hosts.owner_id in (?) AND hosts.owner_type = ‘Usergroup’) OR (hosts.owner_id = ? AND hosts.owner_type = ‘User’))", user.my_usergroups.map(&:id), user.id])
domain_conditions = sanitize_sql_for_conditions([" (hosts.domain_id in (?))",dms = (user.domains).map(&:id)])
compute_resource_conditions = sanitize_sql_for_conditions([" (hosts.compute_resource_id in (?))",(crs = user.compute_resources).map(&:id)])
hostgroup_conditions = sanitize_sql_for_conditions([" (hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])

fact_conditions = “”
@@ -94,6 +95,7 @@ class Host < Puppet::Rails::Host
if user.filtering?
conditions = “#{owner_conditions}” if user.filter_on_owner
(conditions = (user.domains_andor == “and”) ? "(#{conditions}) and #{domain_conditions} " : "#{conditions} or #{domain_conditions} ") unless dms.empty?

 (conditions = (user.compute_resources_andor   == "and") ? "(#{conditions}) and #{compute_resource_conditions} "  : "#{conditions} or #{compute_resource_conditions} ")  unless crs.empty?
 (conditions = (user.hostgroups_andor == "and") ? "(#{conditions}) and #{hostgroup_conditions} " : "#{conditions} or #{hostgroup_conditions} ") unless hgs.empty?
 (conditions = (user.facts_andor      == "and") ? "(#{conditions}) and #{fact_conditions} "      : "#{conditions} or #{fact_conditions} ")      unless ufs.empty?
 conditions.sub!(/\s*\(\)\s*/, "")

diff --git a/app/models/user.rb b/app/models/user.rb
index abe67d4…42917df 100644
— a/app/models/user.rb
+++ b/app/models/user.rb
@@ -15,6 +15,7 @@ class User < ActiveRecord::Base
has_and_belongs_to_many :notices, :join_table => 'user_notices’
has_many :user_roles
has_many :roles, :through => :user_roles

has_and_belongs_to_many :compute_resources, :join_table => "user_compute_resources"
has_and_belongs_to_many :domains, :join_table => "user_domains"
has_and_belongs_to_many :hostgroups, :join_table => "user_hostgroups"
has_many :user_facts, :dependent => :destroy
@@ -36,7 +37,7 @@ class User < ActiveRecord::Base
before_destroy EnsureNotUsedBy.new(:hosts), :ensure_admin_is_not_deleted
validate :name_used_in_a_usergroup, :ensure_admin_is_not_renamed
before_validation :prepare_password

after_destroy Proc.new {|user| user.domains.clear; user.hostgroups.clear}

after_destroy Proc.new {|user| user.compute_resources.clear; user.domains.clear; user.hostgroups.clear}

scoped_search :on => :login, :complete_value => :true
scoped_search :on => :firstname, :complete_value => :true
@@ -153,6 +154,7 @@ class User < ActiveRecord::Base

Returns : Boolean

def filtering?
filter_on_owner or
compute_resources.any? or
domains.any? or
hostgroups.any? or
facts.any?
diff --git a/app/views/compute_resources/vms/index/_libvirt.html.erb b/app/views/compute_resources/vms/index/_libvirt.html.erb
index 901c37a…880a163 100644
— a/app/views/compute_resources/vms/index/_libvirt.html.erb
+++ b/app/views/compute_resources/vms/index/_libvirt.html.erb
@@ -10,7 +10,7 @@

<% @compute_resource.vms.each do |vm| -%> - <%= link_to vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.uuid) %> + <%= link_to_if_authorized vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.uuid) %> <%= vm.cpus %> <%= number_to_human_size vm.memory_size*1024 %> > <%= vm_state(!vm.ready?) %> diff --git a/app/views/compute_resources/vms/index/_ovirt.html.erb b/app/views/compute_resources/vms/index/_ovirt.html.erb index 4964996..03d6e3b 100644 --- a/app/views/compute_resources/vms/index/_ovirt.html.erb +++ b/app/views/compute_resources/vms/index/_ovirt.html.erb @@ -10,7 +10,7 @@ <% @compute_resource.vms.each do |vm| -%> - <%= link_to vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity) %> + <%= link_to_if_authorized vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity) %> <%= vm.cores %> <%= number_to_human_size vm.memory %> > <%= vm_state(!vm.ready?) %> diff --git a/app/views/hosts/_form.html.erb b/app/views/hosts/_form.html.erb index d15dfcf..fdcb622 100644 --- a/app/views/hosts/_form.html.erb +++ b/app/views/hosts/_form.html.erb @@ -25,7 +25,7 @@

<%= text_f f, :name, :class => "input-xlarge", :value => name_field(@host) %> <% libvirt = Hypervisor.first.nil? ? [] : [OpenStruct.new(:to_label => 'Libvirt')] -%> - <%= select_f f, :compute_resource_id, libvirt + ComputeResource.all, :id, :to_label, + <%= select_f f, :compute_resource_id, libvirt + ComputeResource.my_compute_resources, :id, :to_label, { :include_blank => 'Bare Metal' }, {:label => "Deploy on", :disabled => !@host.new_record?, :'data-url' => compute_resource_selected_hosts_path , :onchange => 'computeResourceSelected(this);'} if @host.new_record? || @host.compute_resource_id %> diff --git a/app/views/users/_filters.html.erb b/app/views/users/_filters.html.erb index 41a7113..ea59ab6 100644 --- a/app/views/users/_filters.html.erb +++ b/app/views/users/_filters.html.erb @@ -16,6 +16,15 @@

<% end %>

<%= field_set_tag “Compute resources” do %>
```
 <div class="control-group">
```

   <%= f.select :compute_resources_andor, [["must be", "and"], ["plus all", "or"]], {}, :class => "input-small" %>

```
   <div class="offset1">
```

     <%= edit_habtm @user, ComputeResource %>

```
   </div>
```
```
 </div>
```
<% end %>
<%= field_set_tag “Hostgroup hosts” do %>

<%= f.select :hostgroups_andor, [[“must be”, “and”], [“plus all”, “or”]], {}, :class => “input-small” %>
diff --git a/db/migrate/20120514113900_add_user_compute_resources.rb b/db/migrate/20120514113900_add_user_compute_resources.rb
new file mode 100644
index 0000000…3023a31
— /dev/null
+++ b/db/migrate/20120514113900_add_user_compute_resources.rb
@@ -0,0 +1,14 @@
+# To suport “Edit my compute resource” security access control we need
+# a mechanism to associate a user with some compute resources
+class AddUserComputeResources < ActiveRecord::Migration
def self.up
create_table :user_compute_resources, :id => false do |t|
```
 t.references :user
```
```
 t.references :compute_resource
```
end
end
def self.down
drop_table :user_compute_resources
end
+end
diff --git a/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb b/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
new file mode 100644
index 0000000…05e6727
— /dev/null
+++ b/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
@@ -0,0 +1,9 @@
+class AddComputeResourcesBooleanToUser < ActiveRecord::Migration
def self.up
add_column :users, :compute_resources_andor, :string, :limit => 3, :default => “or”
end
def self.down
remove_column :users, :compute_resources_andor
end
+end
diff --git a/lib/foreman/access_permissions.rb b/lib/foreman/access_permissions.rb
index 3f568e5…9bdcde9 100644
— a/lib/foreman/access_permissions.rb
+++ b/lib/foreman/access_permissions.rb
@@ -23,6 +23,20 @@ Foreman::AccessControl.map do |map|
map.permission :destroy_bookmarks, {:bookmarks => [:destroy]}
end
map.security_block :compute_resources do |map|
map.permission :view_compute_resources, {:compute_resources => [:index, :show]}
map.permission :create_compute_resources, {:compute_resources => [:new, :create]}
map.permission :edit_compute_resources, {:compute_resources => [:edit, :update]}
map.permission :destroy_compute_resources, {:compute_resources => [:destroy]}
end
map.security_block :compute_resources_vms do |map|
map.permission :view_compute_resources_vms, {:compute_resources_vms => [:index, :show]}
map.permission :create_compute_resources_vms, {:compute_resources_vms => [:create, :update]}
map.permission :destroy_compute_resources_vms, {:compute_resources_vms => [:destroy]}
map.permission :power_compute_resources_vms, {:compute_resources_vms => [:power]}
end
map.security_block :config_templates do |map|
map.permission :view_templates, {:config_templates => [:index, :show]}
map.permission :create_templates, {:config_templates => [:new, :create]}
@@ -75,6 +89,8 @@ Foreman::AccessControl.map do |map|
:update_multiple_hostgroup, :update_multiple_parameters, :toggle_manage]}
map.permission :destroy_hosts, {:hosts => [:destroy, :multiple_actions, :reset_multiple, :multiple_destroy, :submit_multiple_destroy]}
map.permission :build_hosts, {:hosts => [:setBuild, :cancelBuild]}
map.permission :power_hosts, {:hosts => [:power]}
map.permission :console_hosts, {:hosts => [:console]}
end

map.security_block :host_editing do |map|

1.7.2.5

ohadlevy · May 21, 2012, 6:36pm

> Add some rights to give non admin users so they can perform some normal
> actions on the hosts they own.
>
> Signed-off-by: Olivier Favre <olivier@yakaz.com>
> —
> …/compute_resources/vms_controller.rb | 2 ±
> app/controllers/compute_resources_controller.rb | 22 +++++++++++±
> app/models/compute_resource.rb | 33
> ++++++++++++++++++++
> app/models/host.rb | 2 +
> app/models/user.rb | 4 +±
> …/compute_resources/vms/index/_libvirt.html.erb | 2 ±
> …/compute_resources/vms/index/_ovirt.html.erb | 2 ±
> app/views/hosts/_form.html.erb | 2 ±
> app/views/users/_filters.html.erb | 9 +++++
> …/20120514113900_add_user_compute_resources.rb | 14 ++++++++
> …115814_add_compute_resources_boolean_to_user.rb | 9 +++++
> lib/foreman/access_permissions.rb | 16 +++++++++
> 12 files changed, 111 insertions(+), 6 deletions(-)
> create mode 100644 db/migrate/20120514113900_add_user_compute_resources.rb
> create mode 100644
> db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
>
> diff --git a/app/controllers/compute_resources/vms_controller.rb
> b/app/controllers/compute_resources/vms_controller.rb
> index 370e9ba…890bf76 100644
> — a/app/controllers/compute_resources/vms_controller.rb
> +++ b/app/controllers/compute_resources/vms_controller.rb
> @@ -60,7 +60,7 @@ class ComputeResources::VmsController <
> ApplicationController
> private
>
> def find_compute_resource
> - @compute_resource = ComputeResource.find(params[:compute_resource_id])
> + @compute_resource =
> ComputeResource.my_compute_resources.find(params[:compute_resource_id])
> end
>
> def find_vm
> diff --git a/app/controllers/compute_resources_controller.rb
> b/app/controllers/compute_resources_controller.rb
> index e0adb31…7723e16 100644
> — a/app/controllers/compute_resources_controller.rb
> +++ b/app/controllers/compute_resources_controller.rb
> @@ -5,7 +5,14 @@ class ComputeResourcesController < ApplicationController
> before_filter :find_by_id, :only => %w{show edit update destroy} +
> AJAX_REQUESTS
>
> def index
> - values = ComputeResource.search_for(params[:search], :order =>
> params[:order])
> + begin
> + my_compute_resources = User.current.admin? ? ComputeResource :
> ComputeResource.my_compute_resources
>
do you mind moving the user check into the scope? this would allow us to
use the scope everywhere by default.
also, i think the current scheme of having Host.my_host,
ComputeResource.my_compute_resources etc gets way too verbose, maybe a
simple ComputeResource.mine or something like that would be better?

> + values = my_compute_resources.search_for(params[:search], :order =>
> params[:order])
> + rescue => e
> + error e.to_s
> + values = my_compute_resources.search_for ""
> + end
> +
> respond_to do |format|
> format.html { @compute_resources = values.paginate :page =>
> params[:page] }
> format.json { render :json => values }
> @@ -16,10 +23,21 @@ class ComputeResourcesController <
> ApplicationController
> @compute_resource = ComputeResource.new
> end
>
> + def show
> + auth = User.current.admin? ? true :
> ComputeResource.my_compute_resources.include?(@compute_resource)
> + not_found and return unless auth
> + respond_to do |format|
> + format.html
> + format.json { render :json => @compute_resource }
> + end
> + end
> +
> def create
> if params[:compute_resource].present? &&
> params[:compute_resource][:provider].present?
> @compute_resource = ComputeResource.new_provider
> params[:compute_resource]
> if @compute_resource.save
> + # Add the new compute resource to the user's filters
> + @compute_resource.users << User.current
> process_success
> else
> process_error
> @@ -32,6 +50,8 @@ class ComputeResourcesController < ApplicationController
> end
>
> def edit
> + auth = User.current.admin? ? true :
> ComputeResource.my_compute_resources.include?(@compute_resource)
> + not_found and return unless auth
>
I think this is probably better as a before_filter?

> end
>
> def update
> diff --git a/app/models/compute_resource.rb
> b/app/models/compute_resource.rb
> index b8106b6…5912b3f 100644
> — a/app/models/compute_resource.rb
> +++ b/app/models/compute_resource.rb
> @@ -8,6 +8,7 @@ class ComputeResource < ActiveRecord::Base
> STI_PREFIX= "Foreman::Model"
>
> include Authorization
> + has_and_belongs_to_many :users, :join_table => "user_compute_resources"
> validates_format_of :name, :with => /\A(\S+)\Z/, :message => "can't be
> blank or contain white spaces."
> validates_uniqueness_of :name
> validates_presence_of :provider, :in => PROVIDERS
> @@ -16,6 +17,38 @@ class ComputeResource < ActiveRecord::Base
> before_save :sanitize_url
> has_many :hosts
>
> + default_scope :order => 'LOWER(compute_resources.name)'
> +
> + # returns reports for hosts in the User's filter set
> + scope :my_compute_resources, lambda {
> + user = User.current
> + if user.admin?
> + conditions = { }
>
Oh, you already do that, so i guess the User check in the controllers are
not needed?

> + else
> + conditions = sanitize_sql_for_conditions([" (compute_resources.idin (?))", user.compute_resources.map(&:id)])
> + conditions.sub!(/\s*()\s*/, "")
> + conditions.sub!(/^(?:())?\s?(?:and|or)\s*/, "")
> + conditions.sub!(/(\s*(?:or|and)\s*(/, "((")
> + end
> + {:conditions => conditions}
> + }
> +
>
should be a private method.

> + def enforce_permissions operation
> + # We get called again with the operation being set to create
> + return true if operation == "edit" and new_record?
> + current = User.current
> + if current.allowed_to?("#{operation}_compute_resources".to_sym)
> + # If you can create compute resources then you can create them
> anywhere
> + return true if operation == "create"
> + # edit or delete
> + if current.allowed_to?("#{operation}_compute_resources".to_sym)
> + return true if
> ComputeResource.my_compute_resources(current).include? self
> + end
> + end
> + errors.add :base, "You do not have permission to #{operation} this
> domain"
> + false
> + end
> +
> # allows to create a specific compute class based on the provider.
> def self.new_provider args
> raise "must provider a provider" unless provider = args[:provider]
> diff --git a/app/models/host.rb b/app/models/host.rb
> index 3d2520a…37bb3e2 100644
> — a/app/models/host.rb
> +++ b/app/models/host.rb
> @@ -79,6 +79,7 @@ class Host < Puppet::Rails::Host
> user = User.current
> owner_conditions = sanitize_sql_for_conditions(["((hosts.owner_id
> in (?) AND hosts.owner_type = 'Usergroup') OR (hosts.owner_id = ? AND
> hosts.owner_type = 'User'))", user.my_usergroups.map(&:id), user.id])
> domain_conditions = sanitize_sql_for_conditions([" (hosts.domain_id
> in (?))",dms = (user.domains).map(&:id)])
> + compute_resource_conditions = sanitize_sql_for_conditions(["
> (hosts.compute_resource_id in (?))",(crs =
> user.compute_resources).map(&:id)])
> hostgroup_conditions = sanitize_sql_for_conditions(["
> (hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])
>
> fact_conditions = ""
> @@ -94,6 +95,7 @@ class Host < Puppet::Rails::Host
> if user.filtering?
> conditions = "#{owner_conditions}"
>
> if user.filter_on_owner
> (conditions = (user.domains_andor == "and") ? "(#{conditions})
> and #{domain_conditions} " : "#{conditions} or #{domain_conditions} ")
> unless dms.empty?
> + (conditions = (user.compute_resources_andor == "and") ?
> "(#{conditions}) and #{compute_resource_conditions} " : "#{conditions} or
> #{compute_resource_conditions} ") unless crs.empty?
> (conditions = (user.hostgroups_andor == "and") ? "(#{conditions})
> and #{hostgroup_conditions} " : "#{conditions} or #{hostgroup_conditions}
> ") unless hgs.empty?
> (conditions = (user.facts_andor == "and") ? "(#{conditions})
> and #{fact_conditions} " : "#{conditions} or #{fact_conditions} ")
> unless ufs.empty?
> conditions.sub!(/\s*()\s*/, "")
> diff --git a/app/models/user.rb b/app/models/user.rb
> index abe67d4…42917df 100644
> — a/app/models/user.rb
> +++ b/app/models/user.rb
> @@ -15,6 +15,7 @@ class User < ActiveRecord::Base
> has_and_belongs_to_many :notices, :join_table => 'user_notices'
> has_many :user_roles
> has_many :roles, :through => :user_roles
> + has_and_belongs_to_many :compute_resources, :join_table =>
> "user_compute_resources"
> has_and_belongs_to_many :domains, :join_table => "user_domains"
> has_and_belongs_to_many :hostgroups, :join_table => "user_hostgroups"
> has_many :user_facts, :dependent => :destroy
> @@ -36,7 +37,7 @@ class User < ActiveRecord::Base
> before_destroy EnsureNotUsedBy.new(:hosts), :ensure_admin_is_not_deleted
> validate :name_used_in_a_usergroup, :ensure_admin_is_not_renamed
> before_validation :prepare_password
> - after_destroy Proc.new {|user| user.domains.clear;
> user.hostgroups.clear}
> + after_destroy Proc.new {|user| user.compute_resources.clear;
> user.domains.clear; user.hostgroups.clear}
>
> scoped_search :on => :login, :complete_value => :true
> scoped_search :on => :firstname, :complete_value => :true
> @@ -153,6 +154,7 @@ class User < ActiveRecord::Base
> # Returns : Boolean
> def filtering?
> filter_on_owner or
> + compute_resources.any? or
> domains.any? or
> hostgroups.any? or
> facts.any?
> diff --git a/app/views/compute_resources/vms/index/_libvirt.html.erb
> b/app/views/compute_resources/vms/index/_libvirt.html.erb
> index 901c37a…880a163 100644
> — a/app/views/compute_resources/vms/index/_libvirt.html.erb
> +++ b/app/views/compute_resources/vms/index/_libvirt.html.erb
> @@ -10,7 +10,7 @@
> </tr>
> <% @compute_resource.vms.each do |vm| -%>
> <tr>
> - <td><%= link_to vm.name,
> hash_for_compute_resource_vm_path(:compute_resource_id =>
> @compute_resource, :id => vm.uuid) %></td>
> + <td><%= link_to_if_authorized vm.name,
> hash_for_compute_resource_vm_path(:compute_resource_id =>
> @compute_resource, :id => vm.uuid) %></td>
> <td><%= vm.cpus %></td>
> <td> <%= number_to_human_size vm.memory_size*1024 %> </td>
> <td <%= vm_power_class(vm.ready?)%>> <%= vm_state(!vm.ready?) %>
> </td>
> diff --git a/app/views/compute_resources/vms/index/_ovirt.html.erb
> b/app/views/compute_resources/vms/index/_ovirt.html.erb
> index 4964996…03d6e3b 100644
> — a/app/views/compute_resources/vms/index/_ovirt.html.erb
> +++ b/app/views/compute_resources/vms/index/_ovirt.html.erb
> @@ -10,7 +10,7 @@
> </tr>
> <% @compute_resource.vms.each do |vm| -%>
> <tr>
> - <td><%= link_to vm.name,
> hash_for_compute_resource_vm_path(:compute_resource_id =>
> @compute_resource, :id => vm.identity) %></td>
> + <td><%= link_to_if_authorized vm.name,
> hash_for_compute_resource_vm_path(:compute_resource_id =>
> @compute_resource, :id => vm.identity) %></td>
> <td><%= vm.cores %></td>
> <td> <%= number_to_human_size vm.memory %> </td>
> <td <%= vm_power_class(vm.ready?) %>> <%= vm_state(!vm.ready?) %>
> </td>
> diff --git a/app/views/hosts/_form.html.erb
> b/app/views/hosts/_form.html.erb
> index d15dfcf…fdcb622 100644
> — a/app/views/hosts/_form.html.erb
> +++ b/app/views/hosts/_form.html.erb
> @@ -25,7 +25,7 @@
> <div class="tab-pane active" id="primary">
> <%= text_f f, :name, :class => "input-xlarge", :value =>
> name_field(@host) %>
> <% libvirt = Hypervisor.first.nil? ? [] : [OpenStruct.new(:to_label
> => 'Libvirt')] -%>
> - <%= select_f f, :compute_resource_id, libvirt +
> ComputeResource.all, :id, :to_label,
> + <%= select_f f, :compute_resource_id, libvirt +
> ComputeResource.my_compute_resources, :id, :to_label,
> { :include_blank => 'Bare Metal' },
> {:label => "Deploy on", :disabled => !@host.new_record?,
> :'data-url' => compute_resource_selected_hosts_path ,
> :onchange => 'computeResourceSelected(this);'} if
> @host.new_record? || @host.compute_resource_id %>
> diff --git a/app/views/users/_filters.html.erb
> b/app/views/users/_filters.html.erb
> index 41a7113…ea59ab6 100644
> — a/app/views/users/_filters.html.erb
> +++ b/app/views/users/_filters.html.erb
> @@ -16,6 +16,15 @@
> </div>
> <% end %>
>
> + <%= field_set_tag "Compute resources" do %>
> + <div class="control-group">
> + <%= f.select :compute_resources_andor, [["must be", "and"],
> ["plus all", "or"]], {}, :class => "input-small" %>
> + <div class="offset1">
> + <%= edit_habtm @user, ComputeResource %>
> + </div>
> + </div>
> + <% end %>
> +
> <%= field_set_tag "Hostgroup hosts" do %>
> <div class="control-group">
> <%= f.select :hostgroups_andor, [["must be", "and"], ["plus all",
> "or"]], {}, :class => "input-small" %>
> diff --git a/db/migrate/20120514113900_add_user_compute_resources.rb
> b/db/migrate/20120514113900_add_user_compute_resources.rb
> new file mode 100644
> index 0000000…3023a31
> — /dev/null
> +++ b/db/migrate/20120514113900_add_user_compute_resources.rb
> @@ -0,0 +1,14 @@
> +# To suport "Edit my compute resource" security access control we need
> +# a mechanism to associate a user with some compute resources
> +class AddUserComputeResources < ActiveRecord::Migration
> + def self.up
> + create_table :user_compute_resources, :id => false do |t|
> + t.references :user
> + t.references :compute_resource
> + end
> + end
> +
> + def self.down
> + drop_table :user_compute_resources
> + end
> +end
> diff --git
> a/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
> b/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
> new file mode 100644
> index 0000000…05e6727
> — /dev/null
> +++ b/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
> @@ -0,0 +1,9 @@
> +class AddComputeResourcesBooleanToUser < ActiveRecord::Migration
> + def self.up
> + add_column :users, :compute_resources_andor, :string, :limit => 3,
> :default => "or"
> + end
> +
> + def self.down
> + remove_column :users, :compute_resources_andor
> + end
> +end
> diff --git a/lib/foreman/access_permissions.rb
> b/lib/foreman/access_permissions.rb
> index 3f568e5…9bdcde9 100644
> — a/lib/foreman/access_permissions.rb
> +++ b/lib/foreman/access_permissions.rb
> @@ -23,6 +23,20 @@ Foreman::AccessControl.map do |map|
> map.permission :destroy_bookmarks, {:bookmarks => [:destroy]}
> end
>
> + map.security_block :compute_resources do |map|
> + map.permission :view_compute_resources, {:compute_resources =>
> [:index, :show]}
> + map.permission :create_compute_resources, {:compute_resources =>
> [:new, :create]}
> + map.permission :edit_compute_resources, {:compute_resources =>
> [:edit, :update]}
> + map.permission :destroy_compute_resources, {:compute_resources =>
> [:destroy]}
> + end
> +
> + map.security_block :compute_resources_vms do |map|
> + map.permission :view_compute_resources_vms,
> {:compute_resources_vms => [:index, :show]}
> + map.permission :create_compute_resources_vms,
> {:compute_resources_vms => [:create, :update]}
> + map.permission :destroy_compute_resources_vms,
> {:compute_resources_vms => [:destroy]}
> + map.permission :power_compute_resources_vms,
> {:compute_resources_vms => [:power]}
> + end
> +
> map.security_block :config_templates do |map|
> map.permission :view_templates, {:config_templates => [:index,
> :show]}
> map.permission :create_templates, {:config_templates => [:new,
> :create]}
> @@ -75,6 +89,8 @@ Foreman::AccessControl.map do |map|
> :update_multiple_hostgroup,
> :update_multiple_parameters, :toggle_manage]}
> map.permission :destroy_hosts, {:hosts => [:destroy,
> :multiple_actions, :reset_multiple, :multiple_destroy,
> :submit_multiple_destroy]}
> map.permission :build_hosts, {:hosts => [:setBuild, :cancelBuild]}
> + map.permission :power_hosts, {:hosts => [:power]}
> + map.permission :console_hosts, {:hosts => [:console]}
> end
>
> map.security_block :host_editing do |map|
> –
> 1.7.2.5
>
>
+1, however I'm missing some basic tests around this, any chance you could
add a few?

thanks!

···

On Mon, May 21, 2012 at 5:43 PM, Olivier Favre wrote:

Olivier_Favre · May 23, 2012, 3:05pm

And add some rights to give non admin users so they can perform some normal actions on the hosts they own.
Including some tests.

Signed-off-by: Olivier Favre <olivier@yakaz.com>

···

--- .../compute_resources/vms_controller.rb | 2 +- app/controllers/compute_resources_controller.rb | 24 +++++- app/models/compute_resource.rb | 34 ++++++++ app/models/host.rb | 2 + app/models/user.rb | 4 +- .../compute_resources/vms/index/_libvirt.html.erb | 2 +- .../compute_resources/vms/index/_ovirt.html.erb | 2 +- app/views/hosts/_form.html.erb | 2 +- app/views/users/_filters.html.erb | 9 ++ .../20120514113900_add_user_compute_resources.rb | 14 +++ ...115814_add_compute_resources_boolean_to_user.rb | 9 ++ lib/foreman/access_permissions.rb | 16 ++++ test/fixtures/compute_resources.yml | 29 +++++++- .../compute_resources_controller_test.rb | 24 +++++- test/unit/compute_resource_test.rb | 86 +++++++++++++++++++- 15 files changed, 244 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20120514113900_add_user_compute_resources.rb create mode 100644 db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb

diff --git a/app/controllers/compute_resources/vms_controller.rb b/app/controllers/compute_resources/vms_controller.rb
index 370e9ba…890bf76 100644
— a/app/controllers/compute_resources/vms_controller.rb
+++ b/app/controllers/compute_resources/vms_controller.rb
@@ -60,7 +60,7 @@ class ComputeResources::VmsController < ApplicationController
private

def find_compute_resource

@compute_resource = ComputeResource.find(params[:compute_resource_id])

@compute_resource = ComputeResource.my_compute_resources.find(params[:compute_resource_id])
end

def find_vm
diff --git a/app/controllers/compute_resources_controller.rb b/app/controllers/compute_resources_controller.rb
index e0adb31…09e9327 100644
— a/app/controllers/compute_resources_controller.rb
+++ b/app/controllers/compute_resources_controller.rb
@@ -2,10 +2,16 @@ class ComputeResourcesController < ApplicationController
include Foreman::Controller::AutoCompleteSearch
AJAX_REQUESTS = %w{hardware_profile_selected cluster_selected}
before_filter :ajax_request, :only => AJAX_REQUESTS

before_filter :find_by_id, :only => %w{show edit update destroy} + AJAX_REQUESTS

before_filter :find_by_id, :only => [:show, :edit, :update, :destroy] + AJAX_REQUESTS

def index

values = ComputeResource.search_for(params[:search], :order => params[:order])

begin

 values = ComputeResource.my_compute_resources.search_for(params[:search], :order => params[:order])

rescue => e
```
 error e.to_s
```

 values = ComputeResource.my_compute_resources.search_for ""

end
respond_to do |format|
format.html { @compute_resources = values.paginate :page => params[:page] }
format.json { render :json => values }
@@ -16,10 +22,19 @@ class ComputeResourcesController < ApplicationController
@compute_resource = ComputeResource.new
end
def show
respond_to do |format|
```
 format.html
```

 format.json { render :json => @compute_resource }

end
end
def create
if params[:compute_resource].present? && params[:compute_resource][:provider].present?
@compute_resource = ComputeResource.new_provider params[:compute_resource]
if @compute_resource.save

   # Add the new compute resource to the user's filters

   @compute_resource.users << User.current
   process_success
 else
   process_error

@@ -31,9 +46,6 @@ class ComputeResourcesController < ApplicationController
end
end

def edit
end
def update
if @compute_resource.update_attributes(params[:compute_resource])
process_success
@@ -82,5 +94,7 @@ class ComputeResourcesController < ApplicationController

def find_by_id
@compute_resource = ComputeResource.find(params[:id])

not_found and return unless @compute_resource
deny_access and return unless ComputeResource.my_compute_resources.include?(@compute_resource)
end
end
diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb
index b8106b6…2d271ee 100644
— a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -8,6 +8,7 @@ class ComputeResource < ActiveRecord::Base
STI_PREFIX= “Foreman::Model”

include Authorization

has_and_belongs_to_many :users, :join_table => "user_compute_resources"
validates_format_of :name, :with => /\A(\S+)\Z/, :message => "can’t be blank or contain white spaces."
validates_uniqueness_of :name
validates_presence_of :provider, :in => PROVIDERS
@@ -16,6 +17,39 @@ class ComputeResource < ActiveRecord::Base
before_save :sanitize_url
has_many :hosts
default_scope :order => ‘LOWER(compute_resources.name)’
returns reports for hosts in the User’s filter set
scope :my_compute_resources, lambda {
user = User.current
if user.admin?
```
 conditions = { }
```
else

 conditions = sanitize_sql_for_conditions([" (compute_resources.id in (?))", user.compute_resources.map(&:id)])

```
 conditions.sub!(/\s*\(\)\s*/, "")
```

 conditions.sub!(/^(?:\(\))?\s?(?:and|or)\s*/, "")

 conditions.sub!(/\(\s*(?:or|and)\s*\(/, "((")

end
{:conditions => conditions}
}
private
def enforce_permissions operation
We get called again with the operation being set to create
return true if operation == “edit” and new_record?
current = User.current
if current.allowed_to?("#{operation}_compute_resources".to_sym)

 # If you can create compute resources then you can create them anywhere

```
 return true if operation == "create"
```
```
 # edit or delete
```

 if current.allowed_to?("#{operation}_compute_resources".to_sym)

   return true if ComputeResource.my_compute_resources(current).include? self

```
 end
```
end
errors.add :base, “You do not have permission to #{operation} this domain”
false
end
allows to create a specific compute class based on the provider.

def self.new_provider args
raise “must provider a provider” unless provider = args[:provider]
diff --git a/app/models/host.rb b/app/models/host.rb
index 4f8f199…86f9fb2 100644
— a/app/models/host.rb
+++ b/app/models/host.rb
@@ -83,6 +83,7 @@ class Host < Puppet::Rails::Host

owner_conditions = sanitize_sql_for_conditions(["((hosts.owner_id in (?) AND hosts.owner_type = ‘Usergroup’) OR (hosts.owner_id = ? AND hosts.owner_type = ‘User’))", user.my_usergroups.map(&:id), user.id])
domain_conditions = sanitize_sql_for_conditions([" (hosts.domain_id in (?))",dms = (user.domains).map(&:id)])
compute_resource_conditions = sanitize_sql_for_conditions([" (hosts.compute_resource_id in (?))",(crs = user.compute_resources).map(&:id)])
hostgroup_conditions = sanitize_sql_for_conditions([" (hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])

fact_conditions = “”
@@ -98,6 +99,7 @@ class Host < Puppet::Rails::Host
if user.filtering?
conditions = “#{owner_conditions}” if user.filter_on_owner
(conditions = (user.domains_andor == “and”) ? "(#{conditions}) and #{domain_conditions} " : "#{conditions} or #{domain_conditions} ") unless dms.empty?

 (conditions = (user.compute_resources_andor   == "and") ? "(#{conditions}) and #{compute_resource_conditions} "  : "#{conditions} or #{compute_resource_conditions} ")  unless crs.empty?
 (conditions = (user.hostgroups_andor == "and") ? "(#{conditions}) and #{hostgroup_conditions} " : "#{conditions} or #{hostgroup_conditions} ") unless hgs.empty?
 (conditions = (user.facts_andor      == "and") ? "(#{conditions}) and #{fact_conditions} "      : "#{conditions} or #{fact_conditions} ")      unless ufs.empty?
 conditions.sub!(/\s*\(\)\s*/, "")

diff --git a/app/models/user.rb b/app/models/user.rb
index abe67d4…42917df 100644
— a/app/models/user.rb
+++ b/app/models/user.rb
@@ -15,6 +15,7 @@ class User < ActiveRecord::Base
has_and_belongs_to_many :notices, :join_table => 'user_notices’
has_many :user_roles
has_many :roles, :through => :user_roles

has_and_belongs_to_many :compute_resources, :join_table => "user_compute_resources"
has_and_belongs_to_many :domains, :join_table => "user_domains"
has_and_belongs_to_many :hostgroups, :join_table => "user_hostgroups"
has_many :user_facts, :dependent => :destroy
@@ -36,7 +37,7 @@ class User < ActiveRecord::Base
before_destroy EnsureNotUsedBy.new(:hosts), :ensure_admin_is_not_deleted
validate :name_used_in_a_usergroup, :ensure_admin_is_not_renamed
before_validation :prepare_password

after_destroy Proc.new {|user| user.domains.clear; user.hostgroups.clear}

after_destroy Proc.new {|user| user.compute_resources.clear; user.domains.clear; user.hostgroups.clear}

scoped_search :on => :login, :complete_value => :true
scoped_search :on => :firstname, :complete_value => :true
@@ -153,6 +154,7 @@ class User < ActiveRecord::Base

Returns : Boolean

def filtering?
filter_on_owner or
compute_resources.any? or
domains.any? or
hostgroups.any? or
facts.any?
diff --git a/app/views/compute_resources/vms/index/_libvirt.html.erb b/app/views/compute_resources/vms/index/_libvirt.html.erb
index 901c37a…880a163 100644
— a/app/views/compute_resources/vms/index/_libvirt.html.erb
+++ b/app/views/compute_resources/vms/index/_libvirt.html.erb
@@ -10,7 +10,7 @@

<% @compute_resource.vms.each do |vm| -%> - <%= link_to vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.uuid) %> + <%= link_to_if_authorized vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.uuid) %> <%= vm.cpus %> <%= number_to_human_size vm.memory_size*1024 %> > <%= vm_state(!vm.ready?) %> diff --git a/app/views/compute_resources/vms/index/_ovirt.html.erb b/app/views/compute_resources/vms/index/_ovirt.html.erb index 4964996..03d6e3b 100644 --- a/app/views/compute_resources/vms/index/_ovirt.html.erb +++ b/app/views/compute_resources/vms/index/_ovirt.html.erb @@ -10,7 +10,7 @@ <% @compute_resource.vms.each do |vm| -%> - <%= link_to vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity) %> + <%= link_to_if_authorized vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity) %> <%= vm.cores %> <%= number_to_human_size vm.memory %> > <%= vm_state(!vm.ready?) %> diff --git a/app/views/hosts/_form.html.erb b/app/views/hosts/_form.html.erb index d15dfcf..fdcb622 100644 --- a/app/views/hosts/_form.html.erb +++ b/app/views/hosts/_form.html.erb @@ -25,7 +25,7 @@

<%= text_f f, :name, :class => "input-xlarge", :value => name_field(@host) %> <% libvirt = Hypervisor.first.nil? ? [] : [OpenStruct.new(:to_label => 'Libvirt')] -%> - <%= select_f f, :compute_resource_id, libvirt + ComputeResource.all, :id, :to_label, + <%= select_f f, :compute_resource_id, libvirt + ComputeResource.my_compute_resources, :id, :to_label, { :include_blank => 'Bare Metal' }, {:label => "Deploy on", :disabled => !@host.new_record?, :'data-url' => compute_resource_selected_hosts_path , :onchange => 'computeResourceSelected(this);'} if @host.new_record? || @host.compute_resource_id %> diff --git a/app/views/users/_filters.html.erb b/app/views/users/_filters.html.erb index 41a7113..ea59ab6 100644 --- a/app/views/users/_filters.html.erb +++ b/app/views/users/_filters.html.erb @@ -16,6 +16,15 @@

<% end %>

<%= field_set_tag “Compute resources” do %>
```
 <div class="control-group">
```

   <%= f.select :compute_resources_andor, [["must be", "and"], ["plus all", "or"]], {}, :class => "input-small" %>

```
   <div class="offset1">
```

     <%= edit_habtm @user, ComputeResource %>

```
   </div>
```
```
 </div>
```
<% end %>
<%= field_set_tag “Hostgroup hosts” do %>

<%= f.select :hostgroups_andor, [[“must be”, “and”], [“plus all”, “or”]], {}, :class => “input-small” %>
diff --git a/db/migrate/20120514113900_add_user_compute_resources.rb b/db/migrate/20120514113900_add_user_compute_resources.rb
new file mode 100644
index 0000000…3023a31
— /dev/null
+++ b/db/migrate/20120514113900_add_user_compute_resources.rb
@@ -0,0 +1,14 @@
+# To suport “Edit my compute resource” security access control we need
+# a mechanism to associate a user with some compute resources
+class AddUserComputeResources < ActiveRecord::Migration
def self.up
create_table :user_compute_resources, :id => false do |t|
```
 t.references :user
```
```
 t.references :compute_resource
```
end
end
def self.down
drop_table :user_compute_resources
end
+end
diff --git a/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb b/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
new file mode 100644
index 0000000…05e6727
— /dev/null
+++ b/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
@@ -0,0 +1,9 @@
+class AddComputeResourcesBooleanToUser < ActiveRecord::Migration
def self.up
add_column :users, :compute_resources_andor, :string, :limit => 3, :default => “or”
end
def self.down
remove_column :users, :compute_resources_andor
end
+end
diff --git a/lib/foreman/access_permissions.rb b/lib/foreman/access_permissions.rb
index 3f568e5…9bdcde9 100644
— a/lib/foreman/access_permissions.rb
+++ b/lib/foreman/access_permissions.rb
@@ -23,6 +23,20 @@ Foreman::AccessControl.map do |map|
map.permission :destroy_bookmarks, {:bookmarks => [:destroy]}
end
map.security_block :compute_resources do |map|
map.permission :view_compute_resources, {:compute_resources => [:index, :show]}
map.permission :create_compute_resources, {:compute_resources => [:new, :create]}
map.permission :edit_compute_resources, {:compute_resources => [:edit, :update]}
map.permission :destroy_compute_resources, {:compute_resources => [:destroy]}
end
map.security_block :compute_resources_vms do |map|
map.permission :view_compute_resources_vms, {:compute_resources_vms => [:index, :show]}
map.permission :create_compute_resources_vms, {:compute_resources_vms => [:create, :update]}
map.permission :destroy_compute_resources_vms, {:compute_resources_vms => [:destroy]}
map.permission :power_compute_resources_vms, {:compute_resources_vms => [:power]}
end
map.security_block :config_templates do |map|
map.permission :view_templates, {:config_templates => [:index, :show]}
map.permission :create_templates, {:config_templates => [:new, :create]}
@@ -75,6 +89,8 @@ Foreman::AccessControl.map do |map|
:update_multiple_hostgroup, :update_multiple_parameters, :toggle_manage]}
map.permission :destroy_hosts, {:hosts => [:destroy, :multiple_actions, :reset_multiple, :multiple_destroy, :submit_multiple_destroy]}
map.permission :build_hosts, {:hosts => [:setBuild, :cancelBuild]}
map.permission :power_hosts, {:hosts => [:power]}
map.permission :console_hosts, {:hosts => [:console]}
end

map.security_block :host_editing do |map|
diff --git a/test/fixtures/compute_resources.yml b/test/fixtures/compute_resources.yml
index de924fb…762c183 100644
— a/test/fixtures/compute_resources.yml
+++ b/test/fixtures/compute_resources.yml
@@ -1,6 +1,6 @@

Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html

-one:
+mycompute:
name: MyString
description: MyString
url: qemu://stam/system
@@ -8,3 +8,30 @@ one:
password: MyString
uuid: MyString
type: Foreman::Model::Libvirt
+
+useless:

name: useless
description: useless
url: qemu://stam-useless/system
user: MyString
password: MyString
uuid: useless
type: Foreman::Model::Libvirt

+dummy:

name: dummy
description: dummy
url: qemu://stam-dummy/system
user: MyString
password: MyString
uuid: dummy
type: Foreman::Model::Libvirt

+yourcompute:

name: yourcompute
description: yourcompute
url: qemu://stam-yourcompute/system
user: MyString
password: MyString
uuid: yourcompute
type: Foreman::Model::Libvirt
diff --git a/test/functional/compute_resources_controller_test.rb b/test/functional/compute_resources_controller_test.rb
index 7ccba9b…0bee18f 100644
— a/test/functional/compute_resources_controller_test.rb
+++ b/test/functional/compute_resources_controller_test.rb
@@ -2,12 +2,20 @@ require ‘test_helper’

class ComputeResourcesControllerTest < ActionController::TestCase
setup do

@compute_resource = compute_resources(:one)

@compute_resource = compute_resources(:mycompute)
@your_compute_resource = compute_resources(:yourcompute)
end

test “should get index” do

setup_user
User.as :admin do

 @one.compute_resources = [compute_resources(:mycompute)]

end
get :index, {}, set_session_user
assert_response :success
computes = ActiveSupport::JSON.decode(@response.body)
assert computes.length == 1
assert computes.include? @compute_resource
assert_not_nil assigns(:compute_resources)
end

@@ -40,6 +48,14 @@ class ComputeResourcesControllerTest < ActionController::TestCase
assert_redirected_to compute_resources_path
end

test “should not destroy compute_resource when not permitted” do
assert_difference('ComputeResource.cou

Olivier_Favre · May 23, 2012, 3:21pm

Hello,

Short note about this patch:
The unit tests are ok, the functional ones are all broken… please help!
Moreover, "test_duplicate_names_cannot_exist_in_a_domain" and
"test_should_have_a_reference_id" from test/unit/domain_parameter_test.rb
fail.

However, I need review. We'll be using it soon.
It's important that : a user has access to the compute resource he creates,
can instanciate hosts on it, cannot see/alter compute resources he has no
access to, cannot instanciate hosts on compute resources he has no access
to.

I'll continue working on the functional tests.

Regards.

···

2012/5/23 Olivier Favre

And add some rights to give non admin users so they can perform some
normal actions on the hosts they own.
Including some tests.

Signed-off-by: Olivier Favre olivier@yakaz.com

…/compute_resources/vms_controller.rb | 2 ±
app/controllers/compute_resources_controller.rb | 24 ++++±
app/models/compute_resource.rb | 34 ++++++++
app/models/host.rb | 2 +
app/models/user.rb | 4 ±
…/compute_resources/vms/index/_libvirt.html.erb | 2 ±
…/compute_resources/vms/index/_ovirt.html.erb | 2 ±
app/views/hosts/_form.html.erb | 2 ±
app/views/users/_filters.html.erb | 9 ++
…/20120514113900_add_user_compute_resources.rb | 14 +++
…115814_add_compute_resources_boolean_to_user.rb | 9 ++
lib/foreman/access_permissions.rb | 16 ++++
test/fixtures/compute_resources.yml | 29 ++++++±
…/compute_resources_controller_test.rb | 24 ++++±
test/unit/compute_resource_test.rb | 86
++++++++++++++++++±
15 files changed, 244 insertions(+), 15 deletions(-)
create mode 100644 db/migrate/20120514113900_add_user_compute_resources.rb
create mode 100644
db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb

diff --git a/app/controllers/compute_resources/vms_controller.rb
b/app/controllers/compute_resources/vms_controller.rb
index 370e9ba…890bf76 100644
— a/app/controllers/compute_resources/vms_controller.rb
+++ b/app/controllers/compute_resources/vms_controller.rb
@@ -60,7 +60,7 @@ class ComputeResources::VmsController <
ApplicationController
private

def find_compute_resource

@compute_resource = ComputeResource.find(params[:compute_resource_id])

@compute_resource =
ComputeResource.my_compute_resources.find(params[:compute_resource_id])
end

def find_vm
diff --git a/app/controllers/compute_resources_controller.rb
b/app/controllers/compute_resources_controller.rb
index e0adb31…09e9327 100644
— a/app/controllers/compute_resources_controller.rb
+++ b/app/controllers/compute_resources_controller.rb
@@ -2,10 +2,16 @@ class ComputeResourcesController < ApplicationController
include Foreman::Controller::AutoCompleteSearch
AJAX_REQUESTS = %w{hardware_profile_selected cluster_selected}
before_filter :ajax_request, :only => AJAX_REQUESTS

before_filter :find_by_id, :only => %w{show edit update destroy} +
AJAX_REQUESTS

before_filter :find_by_id, :only => [:show, :edit, :update, :destroy] +
AJAX_REQUESTS

def index

values = ComputeResource.search_for(params[:search], :order =>
params[:order])
begin
 values =
ComputeResource.my_compute_resources.search_for(params[:search], :order =>
params[:order])
rescue => e
 error e.to_s
 values = ComputeResource.my_compute_resources.search_for ""
end

respond_to do |format|
format.html { @compute_resources = values.paginate :page =>
params[:page] }
format.json { render :json => values }
@@ -16,10 +22,19 @@ class ComputeResourcesController <
ApplicationController
@compute_resource = ComputeResource.new
end

def show

respond_to do |format|
 format.html
 format.json { render :json => @compute_resource }
end

end

def create
if params[:compute_resource].present? &&
params[:compute_resource][:provider].present?
@compute_resource = ComputeResource.new_provider
params[:compute_resource]
if @compute_resource.save
   # Add the new compute resource to the user's filters
   @compute_resource.users << User.current
  process_success
else
  process_error
@@ -31,9 +46,6 @@ class ComputeResourcesController < ApplicationController
end
end

def edit

end

def update
if @compute_resource.update_attributes(params[:compute_resource])
process_success
@@ -82,5 +94,7 @@ class ComputeResourcesController < ApplicationController

def find_by_id
@compute_resource = ComputeResource.find(params[:id])

not_found and return unless @compute_resource

deny_access and return unless
ComputeResource.my_compute_resources.include?(@compute_resource)
end
end
diff --git a/app/models/compute_resource.rb
b/app/models/compute_resource.rb
index b8106b6…2d271ee 100644
— a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -8,6 +8,7 @@ class ComputeResource < ActiveRecord::Base
STI_PREFIX= “Foreman::Model”

include Authorization
has_and_belongs_to_many :users, :join_table => "user_compute_resources"
validates_format_of :name, :with => /\A(\S+)\Z/, :message => "can’t be
blank or contain white spaces."
validates_uniqueness_of :name
validates_presence_of :provider, :in => PROVIDERS
@@ -16,6 +17,39 @@ class ComputeResource < ActiveRecord::Base
before_save :sanitize_url
has_many :hosts

default_scope :order => ‘LOWER(compute_resources.name)’

returns reports for hosts in the User’s filter set

scope :my_compute_resources, lambda {

user = User.current

if user.admin?
 conditions = { }
else
 conditions = sanitize_sql_for_conditions([" (compute_resources.idin (?))", user.compute_resources.map(&:id)])
 conditions.sub!(/\s*\(\)\s*/, "")
 conditions.sub!(/^(?:\(\))?\s?(?:and|or)\s*/, "")
 conditions.sub!(/\(\s*(?:or|and)\s*\(/, "((")
end

{:conditions => conditions}

}

private

def enforce_permissions operation

We get called again with the operation being set to create

return true if operation == “edit” and new_record?

current = User.current

if current.allowed_to?("#{operation}_compute_resources".to_sym)
 # If you can create compute resources then you can create them
anywhere
 return true if operation == "create"
 # edit or delete
 if current.allowed_to?("#{operation}_compute_resources".to_sym)
   return true if
ComputeResource.my_compute_resources(current).include? self
 end
end

errors.add :base, “You do not have permission to #{operation} this
domain”

false

end

allows to create a specific compute class based on the provider.

def self.new_provider args
raise “must provider a provider” unless provider = args[:provider]
diff --git a/app/models/host.rb b/app/models/host.rb
index 4f8f199…86f9fb2 100644
— a/app/models/host.rb
+++ b/app/models/host.rb
@@ -83,6 +83,7 @@ class Host < Puppet::Rails::Host

owner_conditions = sanitize_sql_for_conditions(["((hosts.owner_id
in (?) AND hosts.owner_type = ‘Usergroup’) OR (hosts.owner_id = ? AND
hosts.owner_type = ‘User’))", user.my_usergroups.map(&:id), user.id])
domain_conditions = sanitize_sql_for_conditions([" (hosts.domain_id
in (?))",dms = (user.domains).map(&:id)])

compute_resource_conditions = sanitize_sql_for_conditions(["
(hosts.compute_resource_id in (?))",(crs =
user.compute_resources).map(&:id)])
hostgroup_conditions = sanitize_sql_for_conditions(["
(hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])
fact_conditions = ""
@@ -98,6 +99,7 @@ class Host < Puppet::Rails::Host
if user.filtering?
conditions = “#{owner_conditions}”

if user.filter_on_owner
(conditions = (user.domains_andor == “and”) ? "(#{conditions})
and #{domain_conditions} " : "#{conditions} or #{domain_conditions} ")
unless dms.empty?
 (conditions = (user.compute_resources_andor   == "and") ?
"(#{conditions}) and #{compute_resource_conditions} " : "#{conditions} or
#{compute_resource_conditions} ") unless crs.empty?
(conditions = (user.hostgroups_andor == “and”) ? "(#{conditions})
and #{hostgroup_conditions} " : “#{conditions} or #{hostgroup_conditions}
”) unless hgs.empty?
(conditions = (user.facts_andor == “and”) ? "(#{conditions})
and #{fact_conditions} " : "#{conditions} or #{fact_conditions} ")
unless ufs.empty?
conditions.sub!(/\s*()\s*/, “”)
diff --git a/app/models/user.rb b/app/models/user.rb
index abe67d4…42917df 100644
— a/app/models/user.rb
+++ b/app/models/user.rb
@@ -15,6 +15,7 @@ class User < ActiveRecord::Base
has_and_belongs_to_many :notices, :join_table => 'user_notices’
has_many :user_roles
has_many :roles, :through => :user_roles

has_and_belongs_to_many :compute_resources, :join_table =>
"user_compute_resources"
has_and_belongs_to_many :domains, :join_table => "user_domains"
has_and_belongs_to_many :hostgroups, :join_table => "user_hostgroups"
has_many :user_facts, :dependent => :destroy
@@ -36,7 +37,7 @@ class User < ActiveRecord::Base
before_destroy EnsureNotUsedBy.new(:hosts), :ensure_admin_is_not_deleted
validate :name_used_in_a_usergroup, :ensure_admin_is_not_renamed
before_validation :prepare_password

after_destroy Proc.new {|user| user.domains.clear;
user.hostgroups.clear}

after_destroy Proc.new {|user| user.compute_resources.clear;
user.domains.clear; user.hostgroups.clear}

scoped_search :on => :login, :complete_value => :true
scoped_search :on => :firstname, :complete_value => :true
@@ -153,6 +154,7 @@ class User < ActiveRecord::Base

Returns : Boolean

def filtering?
filter_on_owner or

compute_resources.any? or
domains.any? or
hostgroups.any? or
facts.any?
diff --git a/app/views/compute_resources/vms/index/_libvirt.html.erb
b/app/views/compute_resources/vms/index/_libvirt.html.erb
index 901c37a…880a163 100644
— a/app/views/compute_resources/vms/index/_libvirt.html.erb
+++ b/app/views/compute_resources/vms/index/_libvirt.html.erb
@@ -10,7 +10,7 @@

<% @compute_resource.vms.each do |vm| -%> - <%= link_to vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.uuid) %> + <%= link_to_if_authorized vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.uuid) %> <%= vm.cpus %> <%= number_to_human_size vm.memory_size*1024 %> > <%= vm_state(!vm.ready?) %> diff --git a/app/views/compute_resources/vms/index/_ovirt.html.erb b/app/views/compute_resources/vms/index/_ovirt.html.erb index 4964996..03d6e3b 100644 --- a/app/views/compute_resources/vms/index/_ovirt.html.erb +++ b/app/views/compute_resources/vms/index/_ovirt.html.erb @@ -10,7 +10,7 @@ <% @compute_resource.vms.each do |vm| -%> - <%= link_to vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity) %> + <%= link_to_if_authorized vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity) %> <%= vm.cores %> <%= number_to_human_size vm.memory %> > <%= vm_state(!vm.ready?) %> diff --git a/app/views/hosts/_form.html.erb b/app/views/hosts/_form.html.erb index d15dfcf..fdcb622 100644 --- a/app/views/hosts/_form.html.erb +++ b/app/views/hosts/_form.html.erb @@ -25,7 +25,7 @@
<%= text_f f, :name, :class => "input-xlarge", :value => name_field(@host) %> <% libvirt = Hypervisor.first.nil? ? [] : [OpenStruct.new(:to_label => 'Libvirt')] -%> - <%= select_f f, :compute_resource_id, libvirt + ComputeResource.all, :id, :to_label, + <%= select_f f, :compute_resource_id, libvirt + ComputeResource.my_compute_resources, :id, :to_label, { :include_blank => 'Bare Metal' }, {:label => "Deploy on", :disabled => !@host.new_record?, :'data-url' => compute_resource_selected_hosts_path , :onchange => 'computeResourceSelected(this);'} if @host.new_record? || @host.compute_resource_id %> diff --git a/app/views/users/_filters.html.erb b/app/views/users/_filters.html.erb index 41a7113..ea59ab6 100644 --- a/app/views/users/_filters.html.erb +++ b/app/views/users/_filters.html.erb @@ -16,6 +16,15 @@
<% end %>
<%= field_set_tag “Compute resources” do %>
 <div class="control-group">
   <%= f.select :compute_resources_andor, [["must be", "and"],
[“plus all”, “or”]], {}, :class => “input-small” %>
   <div class="offset1">
     <%= edit_habtm @user, ComputeResource %>
   </div>
 </div>
<% end %>

<%= field_set_tag “Hostgroup hosts” do %>

<%= f.select :hostgroups_andor, [[“must be”, “and”], [“plus all”,
“or”]], {}, :class => “input-small” %>
diff --git a/db/migrate/20120514113900_add_user_compute_resources.rb
b/db/migrate/20120514113900_add_user_compute_resources.rb
new file mode 100644
index 0000000…3023a31
— /dev/null
+++ b/db/migrate/20120514113900_add_user_compute_resources.rb
@@ -0,0 +1,14 @@
+# To suport “Edit my compute resource” security access control we need
+# a mechanism to associate a user with some compute resources
+class AddUserComputeResources < ActiveRecord::Migration

def self.up

create_table :user_compute_resources, :id => false do |t|
 t.references :user
 t.references :compute_resource
end

end

def self.down

drop_table :user_compute_resources

end
+end
diff --git
a/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
b/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
new file mode 100644
index 0000000…05e6727
— /dev/null
+++ b/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
@@ -0,0 +1,9 @@
+class AddComputeResourcesBooleanToUser < ActiveRecord::Migration

def self.up

add_column :users, :compute_resources_andor, :string, :limit => 3,
:default => “or”

end

def self.down

remove_column :users, :compute_resources_andor

end
+end
diff --git a/lib/foreman/access_permissions.rb
b/lib/foreman/access_permissions.rb
index 3f568e5…9bdcde9 100644
— a/lib/foreman/access_permissions.rb
+++ b/lib/foreman/access_permissions.rb
@@ -23,6 +23,20 @@ Foreman::AccessControl.map do |map|
map.permission :destroy_bookmarks, {:bookmarks => [:destroy]}
end

map.security_block :compute_resources do |map|

map.permission :view_compute_resources, {:compute_resources =>
[:index, :show]}

map.permission :create_compute_resources, {:compute_resources =>
[:new, :create]}

map.permission :edit_compute_resources, {:compute_resources =>
[:edit, :update]}

map.permission :destroy_compute_resources, {:compute_resources =>
[:destroy]}

end

map.security_block :compute_resources_vms do |map|

map.permission :view_compute_resources_vms,
{:compute_resources_vms => [:index, :show]}

map.permission :create_compute_resources_vms,
{:compute_resources_vms => [:create, :update]}

map.permission :destroy_compute_resources_vms,
{:compute_resources_vms => [:destroy]}

map.permission :power_compute_resources_vms,
{:compute_resources_vms => [:power]}

end

map.security_block :config_templates do |map|
map.permission :view_templates, {:config_templates => [:index,
:show]}
map.permission :create_templates, {:config_templates => [:new,
:create]}
@@ -75,6 +89,8 @@ Foreman::AccessControl.map do |map|
:update_multiple_hostgroup,
:update_multiple_parameters, :toggle_manage]}
map.permission :destroy_hosts, {:hosts => [:destroy,
:multiple_actions, :reset_multiple, :multiple_destroy,
:submit_multiple_destroy]}
map.permission :build_hosts, {:hosts => [:setBuild, :cancelBuild]}

map.permission :power_hosts, {:hosts => [:power]}

map.permission :console_hosts, {:hosts => [:console]}
end

map.security_block :host_editing do |map|
diff --git a/test/fixtures/compute_resources.yml
b/test/fixtures/compute_resources.yml
index de924fb…762c183 100644
— a/test/fixtures/compute_resources.yml
+++ b/test/fixtures/compute_resources.yml
@@ -1,6 +1,6 @@

Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html

-one:
+mycompute:
name: MyString
description: MyString
url: qemu://stam/system
@@ -8,3 +8,30 @@ one:
password: MyString
uuid: MyString
type: Foreman::Model::Libvirt
+
+useless:

name: useless

description: useless

url: qemu://

Olivier_Favre · May 24, 2012, 8:17am

When I checkout the parent commit of that patch
(c9579050559e837a91713354ae269d96fd0d10a4 Greg Sutcliffe: "Refactor
my_hosts and my_facts scopes to include auth checks" in my case), and
run “rake test”, everything test pass.

When I apply the patch and run “rake test”, the two previously stated
tests of domain fail.
However, when I run “RAILS_ENV=test ruby
unit/domain_parameter_test.rb” I see 3 errors
(test_duplicate_names_can_exist_in_different_domains,
test_should_have_a_reference_id,
test_duplicate_names_cannot_exist_in_a_domain) in the same test file.
All of which are due do User.current being nil.

Here's a short fix:

commit 42b8bde05c8c0196081c0b1985de0a0f8becfaba
Author: Olivier Favre <olivier@yakaz.com>

Fix self-breaking test

diff --git a/test/unit/domain_parameter_test.rb
b/test/unit/domain_parameter_test.rb
index 8408cb4…af41117 100644
— a/test/unit/domain_parameter_test.rb
+++ b/test/unit/domain_parameter_test.rb
@@ -5,12 +5,14 @@ class DomainParameterTest < ActiveSupport::TestCase
parameter = DomainParameter.create(:name => "value", :value => "value")
assert !parameter.save

setup_user "create"
domain = Domain.find_or_create_by_name("domain")
parameter.reference_id = domain.id
assert parameter.save
end

test "duplicate names cannot exist in a domain" do

setup_user "create"
parameter1 = DomainParameter.create :name => "some parameter",
:value => "value", :reference_id => Domain.first.id
parameter2 = DomainParameter.create :name => "some parameter",
:value => "value", :reference_id => Domain.first.id
assert !parameter2.valid?
@@ -18,6 +20,7 @@ class DomainParameterTest < ActiveSupport::TestCase
end

test "duplicate names can exist in different domains" do

setup_user "create"
parameter1 = DomainParameter.create :name => "some parameter",
:value => "value", :reference_id => Domain.first.id
parameter2 = DomainParameter.create :name => "some parameter",
:value => "value", :reference_id => Domain.last.id
assert parameter2.valid?

···

Date: Wed May 23 17:43:48 2012 +0200 --

I wonder why those test broke suddenly… is it because of the way of
running tests, or bad test independence?

Best.

2012/5/23 Olivier Favre olivier@yakaz.com

Hello,

Short note about this patch:
The unit tests are ok, the functional ones are all broken… please help!
Moreover, “test_duplicate_names_cannot_exist_in_a_domain” and “test_should_have_a_reference_id” from test/unit/domain_parameter_test.rb fail.

However, I need review. We’ll be using it soon.
It’s important that : a user has access to the compute resource he creates, can instanciate hosts on it, cannot see/alter compute resources he has no access to, cannot instanciate hosts on compute resources he has no access to.

I’ll continue working on the functional tests.

Regards.

2012/5/23 Olivier Favre olivier@yakaz.com
And add some rights to give non admin users so they can perform some normal actions on the hosts they own.
Including some tests.

Signed-off-by: Olivier Favre olivier@yakaz.com

…/compute_resources/vms_controller.rb | 2 ±
app/controllers/compute_resources_controller.rb | 24 ++++±
app/models/compute_resource.rb | 34 ++++++++
app/models/host.rb | 2 +
app/models/user.rb | 4 ±
…/compute_resources/vms/index/_libvirt.html.erb | 2 ±
…/compute_resources/vms/index/_ovirt.html.erb | 2 ±
app/views/hosts/_form.html.erb | 2 ±
app/views/users/_filters.html.erb | 9 ++
…/20120514113900_add_user_compute_resources.rb | 14 +++
…115814_add_compute_resources_boolean_to_user.rb | 9 ++
lib/foreman/access_permissions.rb | 16 ++++
test/fixtures/compute_resources.yml | 29 ++++++±
…/compute_resources_controller_test.rb | 24 ++++±
test/unit/compute_resource_test.rb | 86 ++++++++++++++++++±
15 files changed, 244 insertions(+), 15 deletions(-)
create mode 100644 db/migrate/20120514113900_add_user_compute_resources.rb
create mode 100644 db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb

diff --git a/app/controllers/compute_resources/vms_controller.rb b/app/controllers/compute_resources/vms_controller.rb
index 370e9ba…890bf76 100644
— a/app/controllers/compute_resources/vms_controller.rb
+++ b/app/controllers/compute_resources/vms_controller.rb
@@ -60,7 +60,7 @@ class ComputeResources::VmsController < ApplicationController
private

def find_compute_resource

@compute_resource = ComputeResource.find(params[:compute_resource_id])

@compute_resource = ComputeResource.my_compute_resources.find(params[:compute_resource_id])
end

def find_vm
diff --git a/app/controllers/compute_resources_controller.rb b/app/controllers/compute_resources_controller.rb
index e0adb31…09e9327 100644
— a/app/controllers/compute_resources_controller.rb
+++ b/app/controllers/compute_resources_controller.rb
@@ -2,10 +2,16 @@ class ComputeResourcesController < ApplicationController
include Foreman::Controller::AutoCompleteSearch
AJAX_REQUESTS = %w{hardware_profile_selected cluster_selected}
before_filter :ajax_request, :only => AJAX_REQUESTS

before_filter :find_by_id, :only => %w{show edit update destroy} + AJAX_REQUESTS

before_filter :find_by_id, :only => [:show, :edit, :update, :destroy] + AJAX_REQUESTS

def index

values = ComputeResource.search_for(params[:search], :order => params[:order])
begin
 values = ComputeResource.my_compute_resources.search_for(params[:search], :order => params[:order])
rescue => e
 error e.to_s
 values = ComputeResource.my_compute_resources.search_for ""
end

respond_to do |format|
format.html { @compute_resources = values.paginate :page => params[:page] }
format.json { render :json => values }
@@ -16,10 +22,19 @@ class ComputeResourcesController < ApplicationController
@compute_resource = ComputeResource.new
end

def show

respond_to do |format|
 format.html
 format.json { render :json => @compute_resource }
end

end

def create
if params[:compute_resource].present? && params[:compute_resource][:provider].present?
@compute_resource = ComputeResource.new_provider params[:compute_resource]
if @compute_resource.save
   # Add the new compute resource to the user's filters
   @compute_resource.users << User.current
  process_success
else
  process_error
@@ -31,9 +46,6 @@ class ComputeResourcesController < ApplicationController
end
end

def edit

end

def update
if @compute_resource.update_attributes(params[:compute_resource])
process_success
@@ -82,5 +94,7 @@ class ComputeResourcesController < ApplicationController

def find_by_id
@compute_resource = ComputeResource.find(params[:id])

not_found and return unless @compute_resource

deny_access and return unless ComputeResource.my_compute_resources.include?(@compute_resource)
end
end
diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb
index b8106b6…2d271ee 100644
— a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -8,6 +8,7 @@ class ComputeResource < ActiveRecord::Base
STI_PREFIX= “Foreman::Model”

include Authorization
has_and_belongs_to_many :users, :join_table => "user_compute_resources"
validates_format_of :name, :with => /\A(\S+)\Z/, :message => "can’t be blank or contain white spaces."
validates_uniqueness_of :name
validates_presence_of :provider, :in => PROVIDERS
@@ -16,6 +17,39 @@ class ComputeResource < ActiveRecord::Base
before_save :sanitize_url
has_many :hosts

default_scope :order => ‘LOWER(compute_resources.name)’

returns reports for hosts in the User’s filter set

scope :my_compute_resources, lambda {

user = User.current

if user.admin?
 conditions = { }
else
 conditions = sanitize_sql_for_conditions([" (compute_resources.id in (?))", user.compute_resources.map(&:id)])
 conditions.sub!(/\s*\(\)\s*/, "")
 conditions.sub!(/^(?:\(\))?\s?(?:and|or)\s*/, "")
 conditions.sub!(/\(\s*(?:or|and)\s*\(/, "((")
end

{:conditions => conditions}

}

private

def enforce_permissions operation

We get called again with the operation being set to create

return true if operation == “edit” and new_record?

current = User.current

if current.allowed_to?("#{operation}_compute_resources".to_sym)
 # If you can create compute resources then you can create them anywhere
 return true if operation == "create"
 # edit or delete
 if current.allowed_to?("#{operation}_compute_resources".to_sym)
   return true if ComputeResource.my_compute_resources(current).include? self
 end
end

errors.add :base, “You do not have permission to #{operation} this domain”

false

end

allows to create a specific compute class based on the provider.

def self.new_provider args
raise “must provider a provider” unless provider = args[:provider]
diff --git a/app/models/host.rb b/app/models/host.rb
index 4f8f199…86f9fb2 100644
— a/app/models/host.rb
+++ b/app/models/host.rb
@@ -83,6 +83,7 @@ class Host < Puppet::Rails::Host

owner_conditions = sanitize_sql_for_conditions(["((hosts.owner_id in (?) AND hosts.owner_type = ‘Usergroup’) OR (hosts.owner_id = ? AND hosts.owner_type = ‘User’))", user.my_usergroups.map(&:id), user.id])
domain_conditions = sanitize_sql_for_conditions([" (hosts.domain_id in (?))",dms = (user.domains).map(&:id)])

compute_resource_conditions = sanitize_sql_for_conditions([" (hosts.compute_resource_id in (?))",(crs = user.compute_resources).map(&:id)])
hostgroup_conditions = sanitize_sql_for_conditions([" (hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])
fact_conditions = ""
@@ -98,6 +99,7 @@ class Host < Puppet::Rails::Host
if user.filtering?
conditions = “#{owner_conditions}” if user.filter_on_owner
(conditions = (user.domains_andor == “and”) ? "(#{conditions}) and #{domain_conditions} " : "#{conditions} or #{domain_conditions} ") unless dms.empty?
 (conditions = (user.compute_resources_andor   == "and") ? "(#{conditions}) and #{compute_resource_conditions} "  : "#{conditions} or #{compute_resource_conditions} ")  unless crs.empty?
(conditions = (user.hostgroups_andor == "and") ? "(#{conditions}) and #{hostgroup_conditions} " : "#{conditions} or #{hostgroup_conditions} ") unless hgs.empty?
(conditions = (user.facts_andor      == "and") ? "(#{conditions}) and #{fact_conditions} "      : "#{conditions} or #{fact_conditions} ")      unless ufs.empty?
conditions.sub!(/\s*\(\)\s*/, "")
diff --git a/app/models/user.rb b/app/models/user.rb
index abe67d4…42917df 100644
— a/app/models/user.rb
+++ b/app/models/user.rb
@@ -15,6 +15,7 @@ class User < ActiveRecord::Base
has_and_belongs_to_many :notices, :join_table => 'user_notices’
has_many :user_roles
has_many :roles, :through => :user_roles

has_and_belongs_to_many :compute_resources, :join_table => "user_compute_resources"
has_and_belongs_to_many :domains, :join_table => "user_domains"
has_and_belongs_to_many :hostgroups, :join_table => "user_hostgroups"
has_many :user_facts, :dependent => :destroy
@@ -36,7 +37,7 @@ class User < ActiveRecord::Base
before_destroy EnsureNotUsedBy.new(:hosts), :ensure_admin_is_not_deleted
validate :name_used_in_a_usergroup, :ensure_admin_is_not_renamed
before_validation :prepare_password

after_destroy Proc.new {|user| user.domains.clear; user.hostgroups.clear}

after_destroy Proc.new {|user| user.compute_resources.clear; user.domains.clear; user.hostgroups.clear}

scoped_search :on => :login, :complete_value => :true
scoped_search :on => :firstname, :complete_value => :true
@@ -153,6 +154,7 @@ class User < ActiveRecord::Base

Returns : Boolean

def filtering?
filter_on_owner or

compute_resources.any? or
domains.any? or
hostgroups.any? or
facts.any?
diff --git a/app/views/compute_resources/vms/index/_libvirt.html.erb b/app/views/compute_resources/vms/index/_libvirt.html.erb
index 901c37a…880a163 100644
— a/app/views/compute_resources/vms/index/_libvirt.html.erb
+++ b/app/views/compute_resources/vms/index/_libvirt.html.erb
@@ -10,7 +10,7 @@

<% @compute_resource.vms.each do |vm| -%> - <%= link_to vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.uuid) %> + <%= link_to_if_authorized vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.uuid) %> <%= vm.cpus %> <%= number_to_human_size vm.memory_size*1024 %> > <%= vm_state(!vm.ready?) %> diff --git a/app/views/compute_resources/vms/index/_ovirt.html.erb b/app/views/compute_resources/vms/index/_ovirt.html.erb index 4964996..03d6e3b 100644 --- a/app/views/compute_resources/vms/index/_ovirt.html.erb +++ b/app/views/compute_resources/vms/index/_ovirt.html.erb @@ -10,7 +10,7 @@ <% @compute_resource.vms.each do |vm| -%> - <%= link_to vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity) %> + <%= link_to_if_authorized vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity) %> <%= vm.cores %> <%= number_to_human_size vm.memory %> > <%= vm_state(!vm.ready?) %> diff --git a/app/views/hosts/_form.html.erb b/app/views/hosts/_form.html.erb index d15dfcf..fdcb622 100644 --- a/app/views/hosts/_form.html.erb +++ b/app/views/hosts/_form.html.erb @@ -25,7 +25,7 @@
<%= text_f f, :name, :class => "input-xlarge", :value => name_field(@host) %> <% libvirt = Hypervisor.first.nil? ? [] : [OpenStruct.new(:to_label => 'Libvirt')] -%> - <%= select_f f, :compute_resource_id, libvirt + ComputeResource.all, :id, :to_label, + <%= select_f f, :compute_resource_id, libvirt + ComputeResource.my_compute_resources, :id, :to_label, { :include_blank => 'Bare Metal' }, {:label => "Deploy on", :disabled => !@host.new_record?, :'data-url' => compute_resource_selected_hosts_path , :onchange => 'computeResourceSelected(this);'} if @host.new_record? || @host.compute_resource_id %> diff --git a/app/views/users/_filters.html.erb b/app/views/users/_filters.html.erb index 41a7113..ea59ab6 100644 --- a/app/views/users/_filters.html.erb +++ b/app/views/users/_filters.html.erb @@ -16,6 +16,15 @@
<% end %>
<%= field_set_tag “Compute resources” do %>
 <div class="control-group">
   <%= f.select :compute_resources_andor, [["must be", "and"], ["plus all", "or"]], {}, :class => "input-small" %>
   <div class="offset1">
     <%= edit_habtm @user, ComputeResource %>
   </div>
 </div>
<% end %>

Olivier_Favre · May 24, 2012, 9:35am

And add some rights to give non admin users so they can perform some normal actions on the hosts they own.
Including some tests.

NOTE: Access control seems broken compared to the previously proposed patch!

Signed-off-by: Olivier Favre <olivier@yakaz.com>

···

--- .../compute_resources/vms_controller.rb | 2 +- app/controllers/compute_resources_controller.rb | 24 +++++- app/models/compute_resource.rb | 37 ++++++++- app/models/host.rb | 2 + app/models/user.rb | 4 +- .../compute_resources/vms/index/_libvirt.html.erb | 2 +- .../compute_resources/vms/index/_ovirt.html.erb | 2 +- app/views/hosts/_form.html.erb | 2 +- app/views/users/_filters.html.erb | 9 ++ .../20120514113900_add_user_compute_resources.rb | 14 +++ ...115814_add_compute_resources_boolean_to_user.rb | 9 ++ lib/foreman/access_permissions.rb | 16 ++++ test/fixtures/compute_resources.yml | 35 +++++++- .../compute_resources_controller_test.rb | 28 ++++++- test/unit/compute_resource_test.rb | 86 +++++++++++++++++++- 15 files changed, 251 insertions(+), 21 deletions(-) create mode 100644 db/migrate/20120514113900_add_user_compute_resources.rb create mode 100644 db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb

diff --git a/app/controllers/compute_resources/vms_controller.rb b/app/controllers/compute_resources/vms_controller.rb
index 370e9ba…890bf76 100644
— a/app/controllers/compute_resources/vms_controller.rb
+++ b/app/controllers/compute_resources/vms_controller.rb
@@ -60,7 +60,7 @@ class ComputeResources::VmsController < ApplicationController
private

def find_compute_resource

@compute_resource = ComputeResource.find(params[:compute_resource_id])

@compute_resource = ComputeResource.my_compute_resources.find(params[:compute_resource_id])
end

def find_vm
diff --git a/app/controllers/compute_resources_controller.rb b/app/controllers/compute_resources_controller.rb
index e0adb31…09e9327 100644
— a/app/controllers/compute_resources_controller.rb
+++ b/app/controllers/compute_resources_controller.rb
@@ -2,10 +2,16 @@ class ComputeResourcesController < ApplicationController
include Foreman::Controller::AutoCompleteSearch
AJAX_REQUESTS = %w{hardware_profile_selected cluster_selected}
before_filter :ajax_request, :only => AJAX_REQUESTS

before_filter :find_by_id, :only => %w{show edit update destroy} + AJAX_REQUESTS

before_filter :find_by_id, :only => [:show, :edit, :update, :destroy] + AJAX_REQUESTS

def index

values = ComputeResource.search_for(params[:search], :order => params[:order])

begin

 values = ComputeResource.my_compute_resources.search_for(params[:search], :order => params[:order])

rescue => e
```
 error e.to_s
```

 values = ComputeResource.my_compute_resources.search_for ""

end
respond_to do |format|
format.html { @compute_resources = values.paginate :page => params[:page] }
format.json { render :json => values }
@@ -16,10 +22,19 @@ class ComputeResourcesController < ApplicationController
@compute_resource = ComputeResource.new
end
def show
respond_to do |format|
```
 format.html
```

 format.json { render :json => @compute_resource }

end
end
def create
if params[:compute_resource].present? && params[:compute_resource][:provider].present?
@compute_resource = ComputeResource.new_provider params[:compute_resource]
if @compute_resource.save

   # Add the new compute resource to the user's filters

   @compute_resource.users << User.current
   process_success
 else
   process_error

@@ -31,9 +46,6 @@ class ComputeResourcesController < ApplicationController
end
end

def edit
end
def update
if @compute_resource.update_attributes(params[:compute_resource])
process_success
@@ -82,5 +94,7 @@ class ComputeResourcesController < ApplicationController

def find_by_id
@compute_resource = ComputeResource.find(params[:id])

not_found and return unless @compute_resource
deny_access and return unless ComputeResource.my_compute_resources.include?(@compute_resource)
end
end
diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb
index b8106b6…8ab0c05 100644
— a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -8,6 +8,7 @@ class ComputeResource < ActiveRecord::Base
STI_PREFIX= “Foreman::Model”

include Authorization

has_and_belongs_to_many :users, :join_table => "user_compute_resources"
validates_format_of :name, :with => /\A(\S+)\Z/, :message => "can’t be blank or contain white spaces."
validates_uniqueness_of :name
validates_presence_of :provider, :in => PROVIDERS
@@ -16,9 +17,25 @@ class ComputeResource < ActiveRecord::Base
before_save :sanitize_url
has_many :hosts
default_scope :order => ‘LOWER(compute_resources.name)’
returns reports for hosts in the User’s filter set
scope :my_compute_resources, lambda {
user = User.current
if user.admin?
```
 conditions = { }
```
else

 conditions = sanitize_sql_for_conditions([" (compute_resources.id in (?))", user.compute_resources.map(&:id)])

```
 conditions.sub!(/\s*\(\)\s*/, "")
```

 conditions.sub!(/^(?:\(\))?\s?(?:and|or)\s*/, "")

 conditions.sub!(/\(\s*(?:or|and)\s*\(/, "((")

end
{:conditions => conditions}
}
allows to create a specific compute class based on the provider.

def self.new_provider args

raise “must provider a provider” unless provider = args[:provider]

raise “must provide a provider” unless provider = args[:provider]
PROVIDERS.each do |p|
return eval("#{STI_PREFIX}::#{p}").new(args) if p.downcase == provider.downcase
end
@@ -155,4 +172,22 @@ class ComputeResource < ActiveRecord::Base
end.compact
end
private
def enforce_permissions operation
We get called again with the operation being set to create
return true if operation == “edit” and new_record?
current = User.current
if current.allowed_to?("#{operation}_compute_resources".to_sym)

 # If you can create compute resources then you can create them anywhere

```
 return true if operation == "create"
```
```
 # edit or delete
```

 if current.allowed_to?("#{operation}_compute_resources".to_sym)

   return true if ComputeResource.my_compute_resources(current).include? self

```
 end
```
end
errors.add :base, “You do not have permission to #{operation} this domain”
false
end

end
diff --git a/app/models/host.rb b/app/models/host.rb
index 4f8f199…86f9fb2 100644
— a/app/models/host.rb
+++ b/app/models/host.rb
@@ -83,6 +83,7 @@ class Host < Puppet::Rails::Host

 owner_conditions     = sanitize_sql_for_conditions(["((hosts.owner_id in (?) AND hosts.owner_type = 'Usergroup') OR (hosts.owner_id = ? AND hosts.owner_type = 'User'))", user.my_usergroups.map(&:id), user.id])
 domain_conditions    = sanitize_sql_for_conditions([" (hosts.domain_id in (?))",dms = (user.domains).map(&:id)])

compute_resource_conditions = sanitize_sql_for_conditions([" (hosts.compute_resource_id in (?))",(crs = user.compute_resources).map(&:id)])
hostgroup_conditions = sanitize_sql_for_conditions([" (hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])

fact_conditions = “”
@@ -98,6 +99,7 @@ class Host < Puppet::Rails::Host
if user.filtering?
conditions = “#{owner_conditions}” if user.filter_on_owner
(conditions = (user.domains_andor == “and”) ? "(#{conditions}) and #{domain_conditions} " : "#{conditions} or #{domain_conditions} ") unless dms.empty?

 (conditions = (user.compute_resources_andor   == "and") ? "(#{conditions}) and #{compute_resource_conditions} "  : "#{conditions} or #{compute_resource_conditions} ")  unless crs.empty?
 (conditions = (user.hostgroups_andor == "and") ? "(#{conditions}) and #{hostgroup_conditions} " : "#{conditions} or #{hostgroup_conditions} ") unless hgs.empty?
 (conditions = (user.facts_andor      == "and") ? "(#{conditions}) and #{fact_conditions} "      : "#{conditions} or #{fact_conditions} ")      unless ufs.empty?
 conditions.sub!(/\s*\(\)\s*/, "")

diff --git a/app/models/user.rb b/app/models/user.rb
index abe67d4…42917df 100644
— a/app/models/user.rb
+++ b/app/models/user.rb
@@ -15,6 +15,7 @@ class User < ActiveRecord::Base
has_and_belongs_to_many :notices, :join_table => 'user_notices’
has_many :user_roles
has_many :roles, :through => :user_roles

has_and_belongs_to_many :compute_resources, :join_table => "user_compute_resources"
has_and_belongs_to_many :domains, :join_table => "user_domains"
has_and_belongs_to_many :hostgroups, :join_table => "user_hostgroups"
has_many :user_facts, :dependent => :destroy
@@ -36,7 +37,7 @@ class User < ActiveRecord::Base
before_destroy EnsureNotUsedBy.new(:hosts), :ensure_admin_is_not_deleted
validate :name_used_in_a_usergroup, :ensure_admin_is_not_renamed
before_validation :prepare_password

after_destroy Proc.new {|user| user.domains.clear; user.hostgroups.clear}

after_destroy Proc.new {|user| user.compute_resources.clear; user.domains.clear; user.hostgroups.clear}

scoped_search :on => :login, :complete_value => :true
scoped_search :on => :firstname, :complete_value => :true
@@ -153,6 +154,7 @@ class User < ActiveRecord::Base

Returns : Boolean

def filtering?
filter_on_owner or
compute_resources.any? or
domains.any? or
hostgroups.any? or
facts.any?
diff --git a/app/views/compute_resources/vms/index/_libvirt.html.erb b/app/views/compute_resources/vms/index/_libvirt.html.erb
index 901c37a…880a163 100644
— a/app/views/compute_resources/vms/index/_libvirt.html.erb
+++ b/app/views/compute_resources/vms/index/_libvirt.html.erb
@@ -10,7 +10,7 @@

<% @compute_resource.vms.each do |vm| -%> - <%= link_to vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.uuid) %> + <%= link_to_if_authorized vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.uuid) %> <%= vm.cpus %> <%= number_to_human_size vm.memory_size*1024 %> > <%= vm_state(!vm.ready?) %> diff --git a/app/views/compute_resources/vms/index/_ovirt.html.erb b/app/views/compute_resources/vms/index/_ovirt.html.erb index 4964996..03d6e3b 100644 --- a/app/views/compute_resources/vms/index/_ovirt.html.erb +++ b/app/views/compute_resources/vms/index/_ovirt.html.erb @@ -10,7 +10,7 @@ <% @compute_resource.vms.each do |vm| -%> - <%= link_to vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity) %> + <%= link_to_if_authorized vm.name, hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity) %> <%= vm.cores %> <%= number_to_human_size vm.memory %> > <%= vm_state(!vm.ready?) %> diff --git a/app/views/hosts/_form.html.erb b/app/views/hosts/_form.html.erb index d15dfcf..fdcb622 100644 --- a/app/views/hosts/_form.html.erb +++ b/app/views/hosts/_form.html.erb @@ -25,7 +25,7 @@

<%= text_f f, :name, :class => "input-xlarge", :value => name_field(@host) %> <% libvirt = Hypervisor.first.nil? ? [] : [OpenStruct.new(:to_label => 'Libvirt')] -%> - <%= select_f f, :compute_resource_id, libvirt + ComputeResource.all, :id, :to_label, + <%= select_f f, :compute_resource_id, libvirt + ComputeResource.my_compute_resources, :id, :to_label, { :include_blank => 'Bare Metal' }, {:label => "Deploy on", :disabled => !@host.new_record?, :'data-url' => compute_resource_selected_hosts_path , :onchange => 'computeResourceSelected(this);'} if @host.new_record? || @host.compute_resource_id %> diff --git a/app/views/users/_filters.html.erb b/app/views/users/_filters.html.erb index 41a7113..ea59ab6 100644 --- a/app/views/users/_filters.html.erb +++ b/app/views/users/_filters.html.erb @@ -16,6 +16,15 @@

<% end %>

<%= field_set_tag “Compute resources” do %>
```
 <div class="control-group">
```

   <%= f.select :compute_resources_andor, [["must be", "and"], ["plus all", "or"]], {}, :class => "input-small" %>

```
   <div class="offset1">
```

     <%= edit_habtm @user, ComputeResource %>

```
   </div>
```
```
 </div>
```
<% end %>
<%= field_set_tag “Hostgroup hosts” do %>

<%= f.select :hostgroups_andor, [[“must be”, “and”], [“plus all”, “or”]], {}, :class => “input-small” %>
diff --git a/db/migrate/20120514113900_add_user_compute_resources.rb b/db/migrate/20120514113900_add_user_compute_resources.rb
new file mode 100644
index 0000000…3023a31
— /dev/null
+++ b/db/migrate/20120514113900_add_user_compute_resources.rb
@@ -0,0 +1,14 @@
+# To suport “Edit my compute resource” security access control we need
+# a mechanism to associate a user with some compute resources
+class AddUserComputeResources < ActiveRecord::Migration
def self.up
create_table :user_compute_resources, :id => false do |t|
```
 t.references :user
```
```
 t.references :compute_resource
```
end
end
def self.down
drop_table :user_compute_resources
end
+end
diff --git a/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb b/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
new file mode 100644
index 0000000…05e6727
— /dev/null
+++ b/db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb
@@ -0,0 +1,9 @@
+class AddComputeResourcesBooleanToUser < ActiveRecord::Migration
def self.up
add_column :users, :compute_resources_andor, :string, :limit => 3, :default => “or”
end
def self.down
remove_column :users, :compute_resources_andor
end
+end
diff --git a/lib/foreman/access_permissions.rb b/lib/foreman/access_permissions.rb
index 3f568e5…9bdcde9 100644
— a/lib/foreman/access_permissions.rb
+++ b/lib/foreman/access_permissions.rb
@@ -23,6 +23,20 @@ Foreman::AccessControl.map do |map|
map.permission :destroy_bookmarks, {:bookmarks => [:destroy]}
end
map.security_block :compute_resources do |map|
map.permission :view_compute_resources, {:compute_resources => [:index, :show]}
map.permission :create_compute_resources, {:compute_resources => [:new, :create]}
map.permission :edit_compute_resources, {:compute_resources => [:edit, :update]}
map.permission :destroy_compute_resources, {:compute_resources => [:destroy]}
end
map.security_block :compute_resources_vms do |map|
map.permission :view_compute_resources_vms, {:compute_resources_vms => [:index, :show]}
map.permission :create_compute_resources_vms, {:compute_resources_vms => [:create, :update]}
map.permission :destroy_compute_resources_vms, {:compute_resources_vms => [:destroy]}
map.permission :power_compute_resources_vms, {:compute_resources_vms => [:power]}
end
map.security_block :config_templates do |map|
map.permission :view_templates, {:config_templates => [:index, :show]}
map.permission :create_templates, {:config_templates => [:new, :create]}
@@ -75,6 +89,8 @@ Foreman::AccessControl.map do |map|
:update_multiple_hostgroup, :update_multiple_parameters, :toggle_manage]}
map.permission :destroy_hosts, {:hosts => [:destroy, :multiple_actions, :reset_multiple, :multiple_destroy, :submit_multiple_destroy]}
map.permission :build_hosts, {:hosts => [:setBuild, :cancelBuild]}
map.permission :power_hosts, {:hosts => [:power]}
map.permission :console_hosts, {:hosts => [:console]}
end

map.security_block :host_editing do |map|
diff --git a/test/fixtures/compute_resources.yml b/test/fixtures/compute_resources.yml
index de924fb…bccf2a0 100644
— a/test/fixtures/compute_resources.yml
+++ b/test/fixtures/compute_resources.yml
@@ -1,10 +1,37 @@

Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html

-one:

name: MyString
description: MyString
+mycompute:

name: mycompute
description: mycompute
url: qemu://stam/system
user: MyString
password: MyString

uuid: MyString

uuid: mycompute
type: Foreman::Model::Libvirt

+useless:

name: useless
description: useless
url: qemu://stam-useless/system
user: MyString
password: MyString
uuid: useless
type: Foreman::Model::Libvirt

+dummy:

name: dummy
description: dummy
url: qemu://stam-dummy/system
user: MyString
password: MyString
uuid: dummy
type: Foreman::Model::Libvirt

+yourcompute:

name: yourcompute
description: yourcompute
url: qemu://stam-yourcompute/system
user: MyString
password: MyString
uuid: yourcompute
type: Foreman::Model::Libvirt
diff --git a/test/functional/compute_resources_controller_test.rb b/test/functional/compute_resources_controller_test.rb
index 7ccba9b…a5a70e4 100644
— a/test/functional/compute_resources_controller_test.rb
+++ b/test/functional/compute_resources_controller_test.rb
@@ -2,12 +2,20 @@ require ‘test_helper’

class ComputeResourcesControllerTest < ActionController::TestCase
setup do

@compute_resource = compute_resources(:one)

@compute_resource = compute_resources(:mycompute)
@your_compute_resource = compute_resources(:yourcompute)
end

test “should get index” do

get :index, {}, set_session_user

setup_user
User.as :admin do

 @one.compute_resources = [compute_resources(:mycompute)]

end
get :i

Olivier_Favre · May 25, 2012, 5:08pm

And add some rights to give non admin users so they can perform some normal actions on the hosts they own.
Including some tests.

unit/domain_parameter_test.rb self-broke… added a tiny fix.
Removing compute_resources/vms nesting, it is such a nightmare for functional tests.

Signed-off-by: Olivier Favre <olivier@yakaz.com>

···

--- app/controllers/application_controller.rb | 2 +- app/controllers/compute_resources_controller.rb | 24 ++- ...ller.rb => compute_resources_vms_controller.rb} | 3 +- app/models/compute_resource.rb | 37 ++++- app/models/host.rb | 2 + app/models/user.rb | 4 +- app/views/compute_resources/vms/show.html.erb | 1 - .../form/_ec2.html.erb | 0 .../form/_libvirt.html.erb | 8 +- .../form/_ovirt.html.erb | 8 +- .../form/libvirt/_network.html.erb | 0 .../form/libvirt/_volume.html.erb | 0 .../form/ovirt/_network.html.erb | 0 .../form/ovirt/_volume.html.erb | 0 .../vms => compute_resources_vms}/index.html.erb | 2 +- .../index/_ec2.html.erb | 0 .../index/_libvirt.html.erb | 2 +- .../index/_ovirt.html.erb | 2 +- .../index/_vmware.html.erb | 0 .../vms => compute_resources_vms}/new.html.erb | 2 +- app/views/compute_resources_vms/show.html.erb | 1 + .../show/_libvirt.html.erb | 0 .../show/_ovirt.html.erb | 0 app/views/hosts/_compute.html.erb | 2 +- app/views/hosts/_form.html.erb | 2 +- app/views/users/_filters.html.erb | 9 + config/routes.rb | 30 ++-- .../20120514113900_add_user_compute_resources.rb | 14 ++ ...115814_add_compute_resources_boolean_to_user.rb | 9 + lib/foreman/access_permissions.rb | 16 ++ test/fixtures/compute_resources.yml | 37 ++++- test/fixtures/roles.yml | 22 +++ .../compute_resources_controller_test.rb | 123 +++++++++++++- .../compute_resources_vms_controller_test.rb | 177 ++++++++++++++++++++ test/unit/compute_resource_test.rb | 86 +++++++++- test/unit/domain_parameter_test.rb | 3 + 36 files changed, 575 insertions(+), 53 deletions(-) rename app/controllers/{compute_resources/vms_controller.rb => compute_resources_vms_controller.rb} (91%) delete mode 100644 app/views/compute_resources/vms/show.html.erb rename app/views/{compute_resources/vms => compute_resources_vms}/form/_ec2.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/_libvirt.html.erb (86%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/_ovirt.html.erb (91%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/libvirt/_network.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/libvirt/_volume.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/ovirt/_network.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/ovirt/_volume.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/index.html.erb (67%) rename app/views/{compute_resources/vms => compute_resources_vms}/index/_ec2.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/index/_libvirt.html.erb (81%) rename app/views/{compute_resources/vms => compute_resources_vms}/index/_ovirt.html.erb (81%) rename app/views/{compute_resources/vms => compute_resources_vms}/index/_vmware.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/new.html.erb (82%) create mode 100644 app/views/compute_resources_vms/show.html.erb rename app/views/{compute_resources/vms => compute_resources_vms}/show/_libvirt.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/show/_ovirt.html.erb (100%) create mode 100644 db/migrate/20120514113900_add_user_compute_resources.rb create mode 100644 db/migrate/20120514115814_add_compute_resources_boolean_to_user.rb create mode 100644 test/functional/compute_resources_vms_controller_test.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 88d2684…e003b0b 100644
— a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -31,7 +31,7 @@ class ApplicationController < ActionController::Base

Authorize the user for the requested action

def authorize(ctrl = params[:controller], action = params[:action])
return true if request.xhr?

allowed = User.current.allowed_to?({:controller => ctrl.gsub(/::/, “_”).underscore, :action => action})

allowed = User.current.allowed_to?({:controller => ctrl.gsub(/::/, “").gsub(///, "”).underscore, :action => action})
allowed ? true : deny_access
end

diff --git a/app/controllers/compute_resources_controller.rb b/app/controllers/compute_resources_controller.rb
index e0adb31…09e9327 100644
— a/app/controllers/compute_resources_controller.rb
+++ b/app/controllers/compute_resources_controller.rb
@@ -2,10 +2,16 @@ class ComputeResourcesController < ApplicationController
include Foreman::Controller::AutoCompleteSearch
AJAX_REQUESTS = %w{hardware_profile_selected cluster_selected}
before_filter :ajax_request, :only => AJAX_REQUESTS

before_filter :find_by_id, :only => %w{show edit update destroy} + AJAX_REQUESTS

before_filter :find_by_id, :only => [:show, :edit, :update, :destroy] + AJAX_REQUESTS

def index

values = ComputeResource.search_for(params[:search], :order => params[:order])

begin

 values = ComputeResource.my_compute_resources.search_for(params[:search], :order => params[:order])

rescue => e
```
 error e.to_s
```

 values = ComputeResource.my_compute_resources.search_for ""

end
respond_to do |format|
format.html { @compute_resources = values.paginate :page => params[:page] }
format.json { render :json => values }
@@ -16,10 +22,19 @@ class ComputeResourcesController < ApplicationController
@compute_resource = ComputeResource.new
end
def show
respond_to do |format|
```
 format.html
```

 format.json { render :json => @compute_resource }

end
end
def create
if params[:compute_resource].present? && params[:compute_resource][:provider].present?
@compute_resource = ComputeResource.new_provider params[:compute_resource]
if @compute_resource.save

   # Add the new compute resource to the user's filters

   @compute_resource.users << User.current
   process_success
 else
   process_error

@@ -31,9 +46,6 @@ class ComputeResourcesController < ApplicationController
end
end

def edit
end
def update
if @compute_resource.update_attributes(params[:compute_resource])
process_success
@@ -82,5 +94,7 @@ class ComputeResourcesController < ApplicationController

def find_by_id
@compute_resource = ComputeResource.find(params[:id])

not_found and return unless @compute_resource
deny_access and return unless ComputeResource.my_compute_resources.include?(@compute_resource)
end
end
diff --git a/app/controllers/compute_resources/vms_controller.rb b/app/controllers/compute_resources_vms_controller.rb
similarity index 91%
rename from app/controllers/compute_resources/vms_controller.rb
rename to app/controllers/compute_resources_vms_controller.rb
index 370e9ba…20cd8df 100644
— a/app/controllers/compute_resources/vms_controller.rb
+++ b/app/controllers/compute_resources_vms_controller.rb
@@ -1,4 +1,4 @@
-class ComputeResources::VmsController < ApplicationController
+class ComputeResourcesVmsController < ApplicationController
before_filter :find_compute_resource
before_filter :find_vm, :only => [:show, :power, :console]

@@ -61,6 +61,7 @@ class ComputeResources::VmsController < ApplicationController

def find_compute_resource
@compute_resource = ComputeResource.find(params[:compute_resource_id])

@compute_resource = ComputeResource.my_compute_resources.find(params[:compute_resource_id]) rescue deny_access
end

def find_vm
diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb
index b8106b6…8ab0c05 100644
— a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -8,6 +8,7 @@ class ComputeResource < ActiveRecord::Base
STI_PREFIX= “Foreman::Model”

include Authorization

has_and_belongs_to_many :users, :join_table => "user_compute_resources"
validates_format_of :name, :with => /\A(\S+)\Z/, :message => "can’t be blank or contain white spaces."
validates_uniqueness_of :name
validates_presence_of :provider, :in => PROVIDERS
@@ -16,9 +17,25 @@ class ComputeResource < ActiveRecord::Base
before_save :sanitize_url
has_many :hosts
default_scope :order => ‘LOWER(compute_resources.name)’
returns reports for hosts in the User’s filter set
scope :my_compute_resources, lambda {
user = User.current
if user.admin?
```
 conditions = { }
```
else

 conditions = sanitize_sql_for_conditions([" (compute_resources.id in (?))", user.compute_resources.map(&:id)])

```
 conditions.sub!(/\s*\(\)\s*/, "")
```

 conditions.sub!(/^(?:\(\))?\s?(?:and|or)\s*/, "")

 conditions.sub!(/\(\s*(?:or|and)\s*\(/, "((")

end
{:conditions => conditions}
}
allows to create a specific compute class based on the provider.

def self.new_provider args

raise “must provider a provider” unless provider = args[:provider]

raise “must provide a provider” unless provider = args[:provider]
PROVIDERS.each do |p|
return eval("#{STI_PREFIX}::#{p}").new(args) if p.downcase == provider.downcase
end
@@ -155,4 +172,22 @@ class ComputeResource < ActiveRecord::Base
end.compact
end
private
def enforce_permissions operation
We get called again with the operation being set to create
return true if operation == “edit” and new_record?
current = User.current
if current.allowed_to?("#{operation}_compute_resources".to_sym)

 # If you can create compute resources then you can create them anywhere

```
 return true if operation == "create"
```
```
 # edit or delete
```

 if current.allowed_to?("#{operation}_compute_resources".to_sym)

   return true if ComputeResource.my_compute_resources(current).include? self

```
 end
```
end
errors.add :base, “You do not have permission to #{operation} this domain”
false
end

end
diff --git a/app/models/host.rb b/app/models/host.rb
index 4f8f199…86f9fb2 100644
— a/app/models/host.rb
+++ b/app/models/host.rb
@@ -83,6 +83,7 @@ class Host < Puppet::Rails::Host

 owner_conditions     = sanitize_sql_for_conditions(["((hosts.owner_id in (?) AND hosts.owner_type = 'Usergroup') OR (hosts.owner_id = ? AND hosts.owner_type = 'User'))", user.my_usergroups.map(&:id), user.id])
 domain_conditions    = sanitize_sql_for_conditions([" (hosts.domain_id in (?))",dms = (user.domains).map(&:id)])

compute_resource_conditions = sanitize_sql_for_conditions([" (hosts.compute_resource_id in (?))",(crs = user.compute_resources).map(&:id)])
hostgroup_conditions = sanitize_sql_for_conditions([" (hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])

fact_conditions = “”
@@ -98,6 +99,7 @@ class Host < Puppet::Rails::Host
if user.filtering?
conditions = “#{owner_conditions}” if user.filter_on_owner
(conditions = (user.domains_andor == “and”) ? "(#{conditions}) and #{domain_conditions} " : "#{conditions} or #{domain_conditions} ") unless dms.empty?

 (conditions = (user.compute_resources_andor   == "and") ? "(#{conditions}) and #{compute_resource_conditions} "  : "#{conditions} or #{compute_resource_conditions} ")  unless crs.empty?
 (conditions = (user.hostgroups_andor == "and") ? "(#{conditions}) and #{hostgroup_conditions} " : "#{conditions} or #{hostgroup_conditions} ") unless hgs.empty?
 (conditions = (user.facts_andor      == "and") ? "(#{conditions}) and #{fact_conditions} "      : "#{conditions} or #{fact_conditions} ")      unless ufs.empty?
 conditions.sub!(/\s*\(\)\s*/, "")

diff --git a/app/models/user.rb b/app/models/user.rb
index abe67d4…42917df 100644
— a/app/models/user.rb
+++ b/app/models/user.rb
@@ -15,6 +15,7 @@ class User < ActiveRecord::Base
has_and_belongs_to_many :notices, :join_table => 'user_notices’
has_many :user_roles
has_many :roles, :through => :user_roles

has_and_belongs_to_many :compute_resources, :join_table => "user_compute_resources"
has_and_belongs_to_many :domains, :join_table => "user_domains"
has_and_belongs_to_many :hostgroups, :join_table => "user_hostgroups"
has_many :user_facts, :dependent => :destroy
@@ -36,7 +37,7 @@ class User < ActiveRecord::Base
before_destroy EnsureNotUsedBy.new(:hosts), :ensure_admin_is_not_deleted
validate :name_used_in_a_usergroup, :ensure_admin_is_not_renamed
before_validation :prepare_password

after_destroy Proc.new {|user| user.domains.clear; user.hostgroups.clear}

after_destroy Proc.new {|user| user.compute_resources.clear; user.domains.clear; user.hostgroups.clear}

scoped_search :on => :login, :complete_value => :true
scoped_search :on => :firstname, :complete_value => :true
@@ -153,6 +154,7 @@ class User < ActiveRecord::Base

Returns : Boolean

def filtering?
filter_on_owner or
compute_resources.any? or
domains.any? or
hostgroups.any? or
facts.any?
diff --git a/app/views/compute_resources/vms/show.html.erb b/app/views/compute_resources/vms/show.html.erb
deleted file mode 100644
index 50124e0…0000000
— a/app/views/compute_resources/vms/show.html.erb
+++ /dev/null
@@ -1 +0,0 @@
-<%= render “compute_resources/vms/show/#{@compute_resource.provider.downcase}” %>
diff --git a/app/views/compute_resources/vms/form/_ec2.html.erb b/app/views/compute_resources_vms/form/_ec2.html.erb
similarity index 100%
rename from app/views/compute_resources/vms/form/_ec2.html.erb
rename to app/views/compute_resources_vms/form/_ec2.html.erb
diff --git a/app/views/compute_resources/vms/form/_libvirt.html.erb b/app/views/compute_resources_vms/form/_libvirt.html.erb
similarity index 86%
rename from app/views/compute_resources/vms/form/_libvirt.html.erb
rename to app/views/compute_resources_vms/form/_libvirt.html.erb
index 6471251…94378f2 100644
— a/app/views/compute_resources/vms/form/_libvirt.html.erb
+++ b/app/views/compute_resources_vms/form/_libvirt.html.erb
@@ -7,10 +7,10 @@

<%= new_child_fields_template(f, :nics, { :object => compute_resource.new_nic, - :partial => 'compute_resources/vms/form/libvirt/network', :form_builder_attrs => { :compute_resource => compute_resource } }) %> + :partial => 'compute_resources_vms/form/libvirt/network', :form_builder_attrs => { :compute_resource => compute_resource } }) %> <%= field_set_tag "Network interfaces", :id => "network_interfaces", :title => 'Networks' do -%> <%= f.fields_for :nics do |i| %> - <%= render 'compute_resources/vms/form/libvirt/network', :f => i, :compute_resource => compute_resource %> + <%= render 'compute_resources_vms/form/libvirt/network', :f => i, :compute_resource => compute_resource %> <% end -%> <%= add_child_link "+ Add Interface", :nics, { :class => "info", :title => 'add new network interface' } %> <% end -%> @@ -20,10 +20,10 @@

<%= new_child_fields_template(f, :volumes, { :object => compute_resource.new_volume, - :partial => 'compute_resources/vms/form/libvirt/volume', :form_builder_attrs => { :compute_resource => compute_resource } }) %> + :partial => 'compute_resources_vms/form/libvirt/volume', :form_builder_attrs => { :compute_resource => compute_resource } }) %> <%= field_set_tag "Storage", :id => "storage_volumes", :title => 'Storage' do -%> <%= f.fields_for :volumes do |i| %> - <%= render 'compute_resources/vms/form/libvirt/volume', :f => i, :compute_resource => compute_resource %> + <%= render 'compute_resources_vms/form/libvirt/volume', :f => i, :compute_resource => compute_resource %> <% end -%> <%= add_child_link "+ Add Volume", :volumes, { :class => "info", :title => 'add new storage volume' } %> <% end -%> diff --git a/app/views/compute_resources/vms/form/_ovirt.html.erb b/app/views/compute_resources_vms/form/_ovirt.html.erb similarity index 91% rename from app/views/compute_resources/vms/form/_ovirt.html.erb rename to app/views/compute_resources_vms/form/_ovirt.html.erb index 12cd39c..ff000ed 100644 --- a/app/views/compute_resources/vms/form/_ovirt.html.erb +++ b/app/views/compute_resources_vms/form/_ovirt.html.erb @@ -20,20 +20,20 @@

<%= new_child_fields_template(f, :interfaces, { :object => compute_resource.new_interface, - :partial => 'compute_resources/vms/form/ovirt/network', :form_builder_attrs => { :clusters => clusters, :compute_resource => compute_resource } }) %> + :partial => 'compute_resources_vms/form/ovirt/network', :form_builder_attrs => { :clusters => clusters, :compute_resource => compute_resource } }) %> <%= field_set_tag "Network interfaces", :id => "network_interfaces", :title => 'Networks' do -%> <%= f.fields_for :interfaces do |i| %> - <%= render 'compute_resources/vms/form/ovirt/network', :f => i, :clusters => clusters, :compute_resource => compute_resource %> + <%= render 'compute_resources_vms/form/ovirt/network', :f => i, :clusters => clusters, :compute_resource => compute_resource %> <% end -%> <

Olivier_Favre · May 29, 2012, 10:24am

And add some rights to give non admin users so they can perform some normal actions on the hosts they own.

unit/domain_parameter_test.rb self-broke… added a tiny fix.
Removing compute_resources/vms nesting, it is such a nightmare for functional tests.

Signed-off-by: Olivier Favre <olivier@yakaz.com>

···

--- app/controllers/application_controller.rb | 2 +- app/controllers/compute_resources_controller.rb | 24 +++- ...ller.rb => compute_resources_vms_controller.rb} | 3 +- app/models/compute_resource.rb | 36 +++++- app/models/host.rb | 16 +- app/models/user.rb | 14 +- app/views/compute_resources/vms/show.html.erb | 1 - .../form/_ec2.html.erb | 0 .../form/_libvirt.html.erb | 8 +- .../form/_ovirt.html.erb | 8 +- .../form/libvirt/_network.html.erb | 0 .../form/libvirt/_volume.html.erb | 0 .../form/ovirt/_network.html.erb | 0 .../form/ovirt/_volume.html.erb | 0 .../vms => compute_resources_vms}/index.html.erb | 2 +- .../index/_ec2.html.erb | 0 .../index/_libvirt.html.erb | 2 +- .../index/_ovirt.html.erb | 2 +- .../index/_vmware.html.erb | 0 .../vms => compute_resources_vms}/new.html.erb | 2 +- app/views/compute_resources_vms/show.html.erb | 1 + .../show/_libvirt.html.erb | 0 .../show/_ovirt.html.erb | 0 app/views/hosts/_compute.html.erb | 2 +- app/views/hosts/_form.html.erb | 2 +- app/views/users/_filters.html.erb | 9 + config/routes.rb | 37 +++-- .../20120529113900_add_user_compute_resources.rb | 14 ++ ...115814_add_compute_resources_boolean_to_user.rb | 9 + lib/foreman/access_permissions.rb | 16 ++ test/fixtures/compute_resources.yml | 18 +++ test/fixtures/roles.yml | 7 + .../compute_resources_controller_test.rb | 123 ++++++++++++++- .../compute_resources_vms_controller_test.rb | 160 ++++++++++++++++++++ test/unit/compute_resource_test.rb | 86 ++++++++++- test/unit/domain_parameter_test.rb | 3 + 36 files changed, 546 insertions(+), 61 deletions(-) rename app/controllers/{compute_resources/vms_controller.rb => compute_resources_vms_controller.rb} (91%) delete mode 100644 app/views/compute_resources/vms/show.html.erb rename app/views/{compute_resources/vms => compute_resources_vms}/form/_ec2.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/_libvirt.html.erb (86%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/_ovirt.html.erb (91%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/libvirt/_network.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/libvirt/_volume.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/ovirt/_network.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/form/ovirt/_volume.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/index.html.erb (67%) rename app/views/{compute_resources/vms => compute_resources_vms}/index/_ec2.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/index/_libvirt.html.erb (81%) rename app/views/{compute_resources/vms => compute_resources_vms}/index/_ovirt.html.erb (81%) rename app/views/{compute_resources/vms => compute_resources_vms}/index/_vmware.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/new.html.erb (82%) create mode 100644 app/views/compute_resources_vms/show.html.erb rename app/views/{compute_resources/vms => compute_resources_vms}/show/_libvirt.html.erb (100%) rename app/views/{compute_resources/vms => compute_resources_vms}/show/_ovirt.html.erb (100%) create mode 100644 db/migrate/20120529113900_add_user_compute_resources.rb create mode 100644 db/migrate/20120529115814_add_compute_resources_boolean_to_user.rb create mode 100644 test/functional/compute_resources_vms_controller_test.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 7386253…fd022a9 100644
— a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -31,7 +31,7 @@ class ApplicationController < ActionController::Base

Authorize the user for the requested action

def authorize(ctrl = params[:controller], action = params[:action])
return true if request.xhr?

allowed = User.current.allowed_to?({:controller => ctrl.gsub(/::/, “_”).underscore, :action => action})

allowed = User.current.allowed_to?({:controller => ctrl.gsub(/::/, “").gsub(///, "”).underscore, :action => action})
allowed ? true : deny_access
end

diff --git a/app/controllers/compute_resources_controller.rb b/app/controllers/compute_resources_controller.rb
index 9ab5128…78b0577 100644
— a/app/controllers/compute_resources_controller.rb
+++ b/app/controllers/compute_resources_controller.rb
@@ -2,10 +2,16 @@ class ComputeResourcesController < ApplicationController
include Foreman::Controller::AutoCompleteSearch
AJAX_REQUESTS = %w{hardware_profile_selected cluster_selected}
before_filter :ajax_request, :only => AJAX_REQUESTS

before_filter :find_by_id, :only => %w{show edit update destroy} + AJAX_REQUESTS

before_filter :find_by_id, :only => [:show, :edit, :update, :destroy] + AJAX_REQUESTS

def index

values = ComputeResource.search_for(params[:search], :order => params[:order])

begin

 values = ComputeResource.my_compute_resources.search_for(params[:search], :order => params[:order])

rescue => e
```
 error e.to_s
```

 values = ComputeResource.my_compute_resources.search_for ""

end
respond_to do |format|
format.html { @compute_resources = values.paginate :page => params[:page] }
format.json { render :json => values }
@@ -16,10 +22,19 @@ class ComputeResourcesController < ApplicationController
@compute_resource = ComputeResource.new
end
def show
respond_to do |format|
```
 format.html
```

 format.json { render :json => @compute_resource }

end
end
def create
if params[:compute_resource].present? && params[:compute_resource][:provider].present?
@compute_resource = ComputeResource.new_provider params[:compute_resource]
if @compute_resource.save

   # Add the new compute resource to the user's filters

   @compute_resource.users << User.current
   process_success :success_redirect => @compute_resource
 else
   process_error

@@ -31,9 +46,6 @@ class ComputeResourcesController < ApplicationController
end
end

def edit
end
def update
if @compute_resource.update_attributes(params[:compute_resource])
process_success
@@ -82,5 +94,7 @@ class ComputeResourcesController < ApplicationController

def find_by_id
@compute_resource = ComputeResource.find(params[:id])

not_found and return unless @compute_resource
deny_access and return unless ComputeResource.my_compute_resources.include?(@compute_resource)
end
end
diff --git a/app/controllers/compute_resources/vms_controller.rb b/app/controllers/compute_resources_vms_controller.rb
similarity index 91%
rename from app/controllers/compute_resources/vms_controller.rb
rename to app/controllers/compute_resources_vms_controller.rb
index 370e9ba…20cd8df 100644
— a/app/controllers/compute_resources/vms_controller.rb
+++ b/app/controllers/compute_resources_vms_controller.rb
@@ -1,4 +1,4 @@
-class ComputeResources::VmsController < ApplicationController
+class ComputeResourcesVmsController < ApplicationController
before_filter :find_compute_resource
before_filter :find_vm, :only => [:show, :power, :console]

@@ -61,6 +61,7 @@ class ComputeResources::VmsController < ApplicationController

def find_compute_resource
@compute_resource = ComputeResource.find(params[:compute_resource_id])

@compute_resource = ComputeResource.my_compute_resources.find(params[:compute_resource_id]) rescue deny_access
end

def find_vm
diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb
index 40a9ce8…18dc6b5 100644
— a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -8,6 +8,7 @@ class ComputeResource < ActiveRecord::Base
STI_PREFIX= “Foreman::Model”

include Authorization

has_and_belongs_to_many :users, :join_table => "user_compute_resources"
validates_format_of :name, :with => /\A(\S+)\Z/, :message => "can’t be blank or contain white spaces."
validates_uniqueness_of :name
validates_presence_of :provider, :in => PROVIDERS
@@ -17,9 +18,24 @@ class ComputeResource < ActiveRecord::Base
has_many :hosts
has_many :images, :dependent => :destroy
default_scope :order => ‘LOWER(compute_resources.name)’
scope :my_compute_resources, lambda {
user = User.current
if user.admin?
```
 conditions = { }
```
else

 conditions = sanitize_sql_for_conditions([" (compute_resources.id in (?))", user.compute_resources.map(&:id)])

```
 conditions.sub!(/\s*\(\)\s*/, "")
```

 conditions.sub!(/^(?:\(\))?\s?(?:and|or)\s*/, "")

 conditions.sub!(/\(\s*(?:or|and)\s*\(/, "((")

end
{:conditions => conditions}
}
allows to create a specific compute class based on the provider.

def self.new_provider args

raise “must provider a provider” unless provider = args[:provider]

raise “must provide a provider” unless provider = args[:provider]
PROVIDERS.each do |p|
return eval("#{STI_PREFIX}::#{p}").new(args) if p.downcase == provider.downcase
end
@@ -165,4 +181,22 @@ class ComputeResource < ActiveRecord::Base
end.compact
end
private
def enforce_permissions operation
We get called again with the operation being set to create
return true if operation == “edit” and new_record?
current = User.current
if current.allowed_to?("#{operation}_compute_resources".to_sym)

 # If you can create compute resources then you can create them anywhere

```
 return true if operation == "create"
```
```
 # edit or delete
```

 if current.allowed_to?("#{operation}_compute_resources".to_sym)

   return true if ComputeResource.my_compute_resources(current).include? self

```
 end
```
end
errors.add :base, “You do not have permission to #{operation} this domain”
false
end

end
diff --git a/app/models/host.rb b/app/models/host.rb
index 170a356…f343517 100644
— a/app/models/host.rb
+++ b/app/models/host.rb
@@ -82,9 +82,10 @@ class Host < Puppet::Rails::Host
user = User.current
return { :conditions => “” } if user.admin? # Admin can see all hosts

owner_conditions = sanitize_sql_for_conditions(["((hosts.owner_id in (?) AND hosts.owner_type = ‘Usergroup’) OR (hosts.owner_id = ? AND hosts.owner_type = ‘User’))", user.my_usergroups.map(&:id), user.id])
domain_conditions = sanitize_sql_for_conditions([" (hosts.domain_id in (?))",dms = (user.domains).map(&:id)])
hostgroup_conditions = sanitize_sql_for_conditions([" (hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])

owner_conditions = sanitize_sql_for_conditions(["((hosts.owner_id in (?) AND hosts.owner_type = ‘Usergroup’) OR (hosts.owner_id = ? AND hosts.owner_type = ‘User’))", user.my_usergroups.map(&:id), user.id])
domain_conditions = sanitize_sql_for_conditions([" (hosts.domain_id in (?))",dms = (user.domains).map(&:id)])
compute_resource_conditions = sanitize_sql_for_conditions([" (hosts.compute_resource_id in (?))",(crs = user.compute_resources).map(&:id)])
hostgroup_conditions = sanitize_sql_for_conditions([" (hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])

fact_conditions = ""
for user_fact in (ufs = user.user_facts)
@@ -97,10 +98,11 @@ class Host < Puppet::Rails::Host

conditions = ""
if user.filtering?

 conditions  = "#{owner_conditions}"                                                                                                            if     user.filter_on_owner

 (conditions = (user.domains_andor    == "and") ? "(#{conditions}) and #{domain_conditions} "    : "#{conditions} or #{domain_conditions} ")    unless dms.empty?

 (conditions = (user.hostgroups_andor == "and") ? "(#{conditions}) and #{hostgroup_conditions} " : "#{conditions} or #{hostgroup_conditions} ") unless hgs.empty?

 (conditions = (user.facts_andor      == "and") ? "(#{conditions}) and #{fact_conditions} "      : "#{conditions} or #{fact_conditions} ")      unless ufs.empty?

 conditions  = "#{owner_conditions}"                                                                                                                                 if     user.filter_on_owner

 (conditions = (user.domains_andor           == "and") ? "(#{conditions}) and #{domain_conditions} "           : "#{conditions} or #{domain_conditions} ")           unless dms.empty?

 (conditions = (user.compute_resources_andor == "and") ? "(#{conditions}) and #{compute_resource_conditions} " : "#{conditions} or #{compute_resource_conditions} ") unless crs.empty?

 (conditions = (user.hostgroups_andor        == "and") ? "(#{conditions}) and #{hostgroup_conditions} "        : "#{conditions} or #{hostgroup_conditions} ")        unless hgs.empty?

 (conditions = (user.facts_andor             == "and") ? "(#{conditions}) and #{fact_conditions} "             : "#{conditions} or #{fact_conditions} ")             unless ufs.empty?
 conditions.sub!(/\s*\(\)\s*/, "")
 conditions.sub!(/^(?:\(\))?\s?(?:and|or)\s*/, "")
 conditions.sub!(/\(\s*(?:or|and)\s*\(/, "((")

diff --git a/app/models/user.rb b/app/models/user.rb
index abe67d4…84ea950 100644
— a/app/models/user.rb
+++ b/app/models/user.rb
@@ -15,8 +15,9 @@ class User < ActiveRecord::Base
has_and_belongs_to_many :notices, :join_table => 'user_notices’
has_many :user_roles
has_many :roles, :through => :user_roles

has_and_belongs_to_many :domains, :join_table => “user_domains”
has_and_belongs_to_many :hostgroups, :join_table => “user_hostgroups”

has_and_belongs_to_many :compute_resources, :join_table => “user_compute_resources”
has_and_belongs_to_many :domains, :join_table => “user_domains”
has_and_belongs_to_many :hostgroups, :join_table => "user_hostgroups"
has_many :user_facts, :dependent => :destroy
has_many :facts, :through => :user_facts, :source => :fact_name

@@ -36,7 +37,7 @@ class User < ActiveRecord::Base
before_destroy EnsureNotUsedBy.new(:hosts), :ensure_admin_is_not_deleted
validate :name_used_in_a_usergroup, :ensure_admin_is_not_renamed
before_validation :prepare_password

after_destroy Proc.new {|user| user.domains.clear; user.hostgroups.clear}

after_destroy Proc.new {|user| user.compute_resources.clear; user.domains.clear; user.hostgroups.clear}

scoped_search :on => :login, :complete_value => :true
scoped_search :on => :firstname, :complete_value => :true
@@ -152,9 +153,10 @@ class User < ActiveRecord::Base

Indicates whether the user has host filtering enabled

Returns : Boolean

def filtering?

filter_on_owner or
domains.any? or
hostgroups.any? or

filter_on_owner or
compute_resources.any? or
domains.any? or
hostgroups.any? or
facts.any?
end

diff --git a/app/views/compute_resources/vms/show.html.erb b/app/views/compute_resources/vms/show.html.erb
deleted file mode 100644
index 50124e0…0000000
— a/app/views/compute_resources/vms/show.html.erb
+++ /dev/null
@@ -1 +0,0 @@
-<%= render “compute_resources/vms/show/#{@compute_resource.provider.downcase}” %>
diff --git a/app/views/compute_resources/vms/form/_ec2.html.erb b/app/views/compute_resources_vms/form/_ec2.html.erb
similarity index 100%
rename from app/views/compute_resources/vms/form/_ec2.html.erb
rename to app/views/compute_resources_vms/form/_ec2.html.erb
diff --git a/app/views/compute_resources/vms/form/_libvirt.html.erb b/app/views/compute_resources_vms/form/_libvirt.html.erb
similarity index 86%
rename from app/views/compute_resources/vms/form/_libvirt.html.erb
rename to app/views/compute_resources_vms/form/_libvirt.html.erb
index 6471251…94378f2 100644
— a/app/views/compute_resources/vms/form/_libvirt.html.erb
+++ b/app/views/compute_resources_vms/form/_libvirt.html.erb
@@ -7,10 +7,10 @@

<%= new_child_fields_template(f, :nics, { :object => compute_resource.new_nic, - :partial => 'compute_resources/vms/form/libvirt/network', :form_builder_attrs => { :compute_resource => compute_resource } }) %> + :partial => 'compute_resources_vms/form/libvirt/network', :form_builder_attrs => { :compute_resource => compute_resource } }) %> <%= field_set_tag "Network interfaces", :id => "network_interfaces", :title => 'Networks' do -%> <%= f.fields_for :nics do |i| %> - <%= render 'compute_resources/vms/form/libvirt/network', :f => i, :compute_resource => compute_resource %> + <%= render 'compute_resources_vms/form/libvirt/network', :f => i, :compute_resource => compute_resource %> <% end -%> <%= add_child_link "+ Add Interface", :nics, { :class => "info", :title => 'add new network interface' } %> <% end -%> @@ -20,10 +20,10 @@

<%= new_child_fields_template(f, :volumes, { :object => compute_resource.new_volume, - :partial => 'compute_resources/vms/form/libvirt/volume', :form_builder_attrs => { :compute_resource => compute_resource } }) %> + :partial => 'compute_resources_vms/form/libvirt/vo

ohadlevy · May 29, 2012, 7:03pm

> And add some rights to give non admin users so they can perform some
> normal actions on the hosts they own.
>
> unit/domain_parameter_test.rb self-broke… added a tiny fix.
> Removing compute_resources/vms nesting, it is such a nightmare for
> functional tests.
>
> Signed-off-by: Olivier Favre <olivier@yakaz.com>
> —
> app/controllers/application_controller.rb | 2 ±
> app/controllers/compute_resources_controller.rb | 24 ++±
> …ller.rb => compute_resources_vms_controller.rb} | 3 ±
> app/models/compute_resource.rb | 36 ++++±
> app/models/host.rb | 16 ±
> app/models/user.rb | 14 ±
> app/views/compute_resources/vms/show.html.erb | 1 -
> …/form/_ec2.html.erb | 0
> …/form/_libvirt.html.erb | 8 ±
> …/form/_ovirt.html.erb | 8 ±
> …/form/libvirt/_network.html.erb | 0
> …/form/libvirt/_volume.html.erb | 0
> …/form/ovirt/_network.html.erb | 0
> …/form/ovirt/_volume.html.erb | 0
> …/vms => compute_resources_vms}/index.html.erb | 2 ±
> …/index/_ec2.html.erb | 0
> …/index/_libvirt.html.erb | 2 ±
> …/index/_ovirt.html.erb | 2 ±
> …/index/_vmware.html.erb | 0
> …/vms => compute_resources_vms}/new.html.erb | 2 ±
> app/views/compute_resources_vms/show.html.erb | 1 +
> …/show/_libvirt.html.erb | 0
> …/show/_ovirt.html.erb | 0
> app/views/hosts/_compute.html.erb | 2 ±
> app/views/hosts/_form.html.erb | 2 ±
> app/views/users/_filters.html.erb | 9 +
> config/routes.rb | 37 ++±-
> …/20120529113900_add_user_compute_resources.rb | 14 ++
> …115814_add_compute_resources_boolean_to_user.rb | 9 +
> lib/foreman/access_permissions.rb | 16 ++
> test/fixtures/compute_resources.yml | 18 +++
> test/fixtures/roles.yml | 7 +
> …/compute_resources_controller_test.rb | 123 +++++++++++++±
> …/compute_resources_vms_controller_test.rb | 160
> ++++++++++++++++++++
> test/unit/compute_resource_test.rb | 86 +++++++++±
> test/unit/domain_parameter_test.rb | 3 +
> 36 files changed, 546 insertions(+), 61 deletions(-)
> rename app/controllers/{compute_resources/vms_controller.rb =>
> compute_resources_vms_controller.rb} (91%)
> delete mode 100644 app/views/compute_resources/vms/show.html.erb
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/form/_ec2.html.erb (100%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/form/_libvirt.html.erb (86%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/form/_ovirt.html.erb (91%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/form/libvirt/_network.html.erb (100%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/form/libvirt/_volume.html.erb (100%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/form/ovirt/_network.html.erb (100%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/form/ovirt/_volume.html.erb (100%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/index.html.erb (67%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/index/_ec2.html.erb (100%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/index/_libvirt.html.erb (81%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/index/_ovirt.html.erb (81%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/index/vmware.html.erb (100%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/new.html.erb (82%)
> create mode 100644 app/views/compute_resources_vms/show.html.erb
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/show/libvirt.html.erb (100%)
> rename app/views/{compute_resources/vms =>
> compute_resources_vms}/show/ovirt.html.erb (100%)
> create mode 100644 db/migrate/20120529113900_add_user_compute_resources.rb
> create mode 100644
> db/migrate/20120529115814_add_compute_resources_boolean_to_user.rb
> create mode 100644
> test/functional/compute_resources_vms_controller_test.rb
>
> diff --git a/app/controllers/application_controller.rb
> b/app/controllers/application_controller.rb
> index 7386253…fd022a9 100644
> — a/app/controllers/application_controller.rb
> +++ b/app/controllers/application_controller.rb
> @@ -31,7 +31,7 @@ class ApplicationController < ActionController::Base
> # Authorize the user for the requested action
> def authorize(ctrl = params[:controller], action = params[:action])
> return true if request.xhr?
> - allowed = User.current.allowed_to?({:controller => ctrl.gsub(/::/,
> "").underscore, :action => action})
> + allowed = User.current.allowed_to?({:controller => ctrl.gsub(/::/,
> "").gsub(///, "").underscore, :action => action})
> allowed ? true : deny_access
>
is this still required after renaming the host? does it hurt if we keep it?

> end
>
> diff --git a/app/controllers/compute_resources_controller.rb
> b/app/controllers/compute_resources_controller.rb
> index 9ab5128…78b0577 100644
> — a/app/controllers/compute_resources_controller.rb
> +++ b/app/controllers/compute_resources_controller.rb
> @@ -2,10 +2,16 @@ class ComputeResourcesController < ApplicationController
> include Foreman::Controller::AutoCompleteSearch
> AJAX_REQUESTS = %w{hardware_profile_selected cluster_selected}
> before_filter :ajax_request, :only => AJAX_REQUESTS
> - before_filter :find_by_id, :only => %w{show edit update destroy} +
> AJAX_REQUESTS
> + before_filter :find_by_id, :only => [:show, :edit, :update, :destroy] +
> AJAX_REQUESTS
>
did it not work? why did you change it? (just wondering)

>
> def index
> - values = ComputeResource.search_for(params[:search], :order =>
> params[:order])
> + begin
> + values =
> ComputeResource.my_compute_resources.search_for(params[:search], :order =>
> params[:order])
> + rescue => e
> + error e.to_s
> + values = ComputeResource.my_compute_resources.search_for ""
> + end
> +
> respond_to do |format|
> format.html { @compute_resources = values.paginate :page =>
> params[:page] }
> format.json { render :json => values }
> @@ -16,10 +22,19 @@ class ComputeResourcesController <
> ApplicationController
> @compute_resource = ComputeResource.new
> end
>
> + def show
> + respond_to do |format|
> + format.html
> + format.json { render :json => @compute_resource }
> + end
> + end
> +
> def create
> if params[:compute_resource].present? &&
> params[:compute_resource][:provider].present?
> @compute_resource = ComputeResource.new_provider
> params[:compute_resource]
> if @compute_resource.save
> + # Add the new compute resource to the user's filters
> + @compute_resource.users << User.current
> process_success :success_redirect => @compute_resource
> else
> process_error
> @@ -31,9 +46,6 @@ class ComputeResourcesController < ApplicationController
> end
> end
>
> - def edit
> - end
>
why did you remove it? i think its a valid controller action?

···

On Tue, May 29, 2012 at 1:24 PM, Olivier Favre wrote:

def update
if @compute_resource.update_attributes(params[:compute_resource])
process_success
@@ -82,5 +94,7 @@ class ComputeResourcesController < ApplicationController

def find_by_id
@compute_resource = ComputeResource.find(params[:id])

not_found and return unless @compute_resource

deny_access and return unless
ComputeResource.my_compute_resources.include?(@compute_resource)
end
end
diff --git a/app/controllers/compute_resources/vms_controller.rb
b/app/controllers/compute_resources_vms_controller.rb
similarity index 91%
rename from app/controllers/compute_resources/vms_controller.rb
rename to app/controllers/compute_resources_vms_controller.rb
index 370e9ba…20cd8df 100644
— a/app/controllers/compute_resources/vms_controller.rb
+++ b/app/controllers/compute_resources_vms_controller.rb
@@ -1,4 +1,4 @@
-class ComputeResources::VmsController < ApplicationController
+class ComputeResourcesVmsController < ApplicationController
before_filter :find_compute_resource
before_filter :find_vm, :only => [:show, :power, :console]

@@ -61,6 +61,7 @@ class ComputeResources::VmsController <
ApplicationController

def find_compute_resource
@compute_resource = ComputeResource.find(params[:compute_resource_id])

@compute_resource =
ComputeResource.my_compute_resources.find(params[:compute_resource_id])
rescue deny_access
end

def find_vm
diff --git a/app/models/compute_resource.rb
b/app/models/compute_resource.rb
index 40a9ce8…18dc6b5 100644
— a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -8,6 +8,7 @@ class ComputeResource < ActiveRecord::Base
STI_PREFIX= “Foreman::Model”

include Authorization
has_and_belongs_to_many :users, :join_table => "user_compute_resources"
validates_format_of :name, :with => /\A(\S+)\Z/, :message => "can’t be
blank or contain white spaces."
validates_uniqueness_of :name
validates_presence_of :provider, :in => PROVIDERS
@@ -17,9 +18,24 @@ class ComputeResource < ActiveRecord::Base
has_many :hosts
has_many :images, :dependent => :destroy

default_scope :order => ‘LOWER(compute_resources.name)’

scope :my_compute_resources, lambda {

user = User.current

if user.admin?
 conditions = { }
else
 conditions = sanitize_sql_for_conditions([" (compute_resources.idin (?))", user.compute_resources.map(&:id)])
 conditions.sub!(/\s*\(\)\s*/, "")
 conditions.sub!(/^(?:\(\))?\s?(?:and|or)\s*/, "")
 conditions.sub!(/\(\s*(?:or|and)\s*\(/, "((")
end

{:conditions => conditions}

}

allows to create a specific compute class based on the provider.

def self.new_provider args
raise “must provider a provider” unless provider = args[:provider]
raise “must provide a provider” unless provider = args[:provider]
PROVIDERS.each do |p|
return eval("#{STI_PREFIX}::#{p}").new(args) if p.downcase ==
provider.downcase
end
@@ -165,4 +181,22 @@ class ComputeResource < ActiveRecord::Base
end.compact
end

private

def enforce_permissions operation

We get called again with the operation being set to create

return true if operation == “edit” and new_record?

current = User.current

if current.allowed_to?("#{operation}_compute_resources".to_sym)
 # If you can create compute resources then you can create them
anywhere
 return true if operation == "create"
 # edit or delete
 if current.allowed_to?("#{operation}_compute_resources".to_sym)
   return true if
ComputeResource.my_compute_resources(current).include? self
 end
end

errors.add :base, “You do not have permission to #{operation} this
domain”

false

end
end
diff --git a/app/models/host.rb b/app/models/host.rb
index 170a356…f343517 100644
— a/app/models/host.rb
+++ b/app/models/host.rb
@@ -82,9 +82,10 @@ class Host < Puppet::Rails::Host
user = User.current
return { :conditions => “” } if user.admin? # Admin can see all hosts

owner_conditions = sanitize_sql_for_conditions(["((hosts.owner_id
in (?) AND hosts.owner_type = ‘Usergroup’) OR (hosts.owner_id = ? AND
hosts.owner_type = ‘User’))", user.my_usergroups.map(&:id), user.id])

domain_conditions = sanitize_sql_for_conditions(["
(hosts.domain_id in (?))",dms = (user.domains).map(&:id)])

hostgroup_conditions = sanitize_sql_for_conditions(["
(hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])

owner_conditions =
sanitize_sql_for_conditions(["((hosts.owner_id in (?) AND hosts.owner_type
= ‘Usergroup’) OR (hosts.owner_id = ? AND hosts.owner_type = ‘User’))",
user.my_usergroups.map(&:id), user.id])

domain_conditions = sanitize_sql_for_conditions(["
(hosts.domain_id in (?))",dms = (user.domains).map(&:id)])

compute_resource_conditions = sanitize_sql_for_conditions(["
(hosts.compute_resource_id in (?))",(crs =
user.compute_resources).map(&:id)])

hostgroup_conditions = sanitize_sql_for_conditions(["
(hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)])
fact_conditions = ""
for user_fact in (ufs = user.user_facts)
@@ -97,10 +98,11 @@ class Host < Puppet::Rails::Host
conditions = ""
if user.filtering?
 conditions  = "#{owner_conditions}"
if user.filter_on_owner
 (conditions = (user.domains_andor    == "and") ? "(#{conditions})
and #{domain_conditions} " : "#{conditions} or #{domain_conditions} ")
unless dms.empty?
 (conditions = (user.hostgroups_andor == "and") ? "(#{conditions})
and #{hostgroup_conditions} " : “#{conditions} or #{hostgroup_conditions}
”) unless hgs.empty?
 (conditions = (user.facts_andor      == "and") ? "(#{conditions})
and #{fact_conditions} " : "#{conditions} or #{fact_conditions} ")
unless ufs.empty?
 conditions  = "#{owner_conditions}"

              if     user.filter_on_owner
 (conditions = (user.domains_andor           == "and") ?
"(#{conditions}) and #{domain_conditions} " : "#{conditions} or
#{domain_conditions} ") unless dms.empty?
 (conditions = (user.compute_resources_andor == "and") ?
"(#{conditions}) and #{compute_resource_conditions} " : "#{conditions} or
#{compute_resource_conditions} ") unless crs.empty?
 (conditions = (user.hostgroups_andor        == "and") ?
"(#{conditions}) and #{hostgroup_conditions} " : "#{conditions} or
#{hostgroup_conditions} ") unless hgs.empty?
 (conditions = (user.facts_andor             == "and") ?
"(#{conditions}) and #{fact_conditions} " : "#{conditions} or
#{fact_conditions} ") unless ufs.empty?
conditions.sub!(/\s*()\s*/, “”)
conditions.sub!(/^(?:())?\s?(?:and|or)\s*/, “”)
conditions.sub!(/(\s*(?:or|and)\s*(/, “((”)
diff --git a/app/models/user.rb b/app/models/user.rb
index abe67d4…84ea950 100644
— a/app/models/user.rb
+++ b/app/models/user.rb
@@ -15,8 +15,9 @@ class User < ActiveRecord::Base
has_and_belongs_to_many :notices, :join_table => 'user_notices’
has_many :user_roles
has_many :roles, :through => :user_roles

has_and_belongs_to_many :domains, :join_table => “user_domains”

has_and_belongs_to_many :hostgroups, :join_table => “user_hostgroups”

has_and_belongs_to_many :compute_resources, :join_table =>
“user_compute_resources”

has_and_belongs_to_many :domains, :join_table =>
“user_domains”

has_and_belongs_to_many :hostgroups, :join_table =>
"user_hostgroups"
has_many :user_facts, :dependent => :destroy
has_many :facts, :through => :user_facts, :source => :fact_name

@@ -36,7 +37,7 @@ class User < ActiveRecord::Base
before_destroy EnsureNotUsedBy.new(:hosts), :ensure_admin_is_not_deleted
validate :name_used_in_a_usergroup, :ensure_admin_is_not_renamed
before_validation :prepare_password

after_destroy Proc.new {|user| user.domains.clear;
user.hostgroups.clear}

after_destroy Proc.new {|user| user.compute_resources.clear;
user.domains.clear; user.hostgroups.clear}

scoped_search :on => :login, :complete_value => :true
scoped_search :on => :firstname, :complete_value => :true
@@ -152,9 +153,10 @@ class User < ActiveRecord::Base

Indicates whether the user has host filtering enabled

Returns : Boolean

def filtering?

filter_on_owner or

domains.any? or

hostgroups.any? or

filter_on_owner or

compute_resources.any? or

domains.any? or

hostgroups.any? or
facts.any?
end

diff --git a/app/views/compute_resources/vms/show.html.erb
b/app/views/compute_resources/vms/show.html.erb
deleted file mode 100644
index 50124e0…0000000
— a/app/views/compute_resources/vms/show.html.erb
+++ /dev/null
@@ -1 +0,0 @@
-<%= render
"compute_resources/vms/show/#{@compute_resource.provider.downcase}" %>
diff --git a/app/views/compute_resources/vms/form/_ec2.html.erb
b/app/views/compute_resources_vms/form/_ec2.html.erb
similarity index 100%
rename from app/views/compute

Olivier_Favre · May 30, 2012, 9:14am

>
>
>>
>> And add some rights to give non admin users so they can perform some
>> normal actions on the hosts they own.
>>
>> unit/domain_parameter_test.rb self-broke… added a tiny fix.
>> Removing compute_resources/vms nesting, it is such a nightmare for
>> functional tests.
>>
>> Signed-off-by: Olivier Favre <olivier@yakaz.com>
>> —
>> app/controllers/application_controller.rb | 2 ±
>> app/controllers/compute_resources_controller.rb | 24 ++±
>> …ller.rb => compute_resources_vms_controller.rb} | 3 ±
>> app/models/compute_resource.rb | 36 ++++±
>> app/models/host.rb | 16 ±
>> app/models/user.rb | 14 ±
>> app/views/compute_resources/vms/show.html.erb | 1 -
>> …/form/_ec2.html.erb | 0
>> …/form/_libvirt.html.erb | 8 ±
>> …/form/_ovirt.html.erb | 8 ±
>> …/form/libvirt/_network.html.erb | 0
>> …/form/libvirt/_volume.html.erb | 0
>> …/form/ovirt/_network.html.erb | 0
>> …/form/ovirt/_volume.html.erb | 0
>> …/vms => compute_resources_vms}/index.html.erb | 2 ±
>> …/index/_ec2.html.erb | 0
>> …/index/_libvirt.html.erb | 2 ±
>> …/index/_ovirt.html.erb | 2 ±
>> …/index/_vmware.html.erb | 0
>> …/vms => compute_resources_vms}/new.html.erb | 2 ±
>> app/views/compute_resources_vms/show.html.erb | 1 +
>> …/show/_libvirt.html.erb | 0
>> …/show/_ovirt.html.erb | 0
>> app/views/hosts/_compute.html.erb | 2 ±
>> app/views/hosts/_form.html.erb | 2 ±
>> app/views/users/_filters.html.erb | 9 +
>> config/routes.rb | 37 ++±-
>> …/20120529113900_add_user_compute_resources.rb | 14 ++
>> …115814_add_compute_resources_boolean_to_user.rb | 9 +
>> lib/foreman/access_permissions.rb | 16 ++
>> test/fixtures/compute_resources.yml | 18 +++
>> test/fixtures/roles.yml | 7 +
>> …/compute_resources_controller_test.rb | 123 +++++++++++++±
>> …/compute_resources_vms_controller_test.rb | 160
>> ++++++++++++++++++++
>> test/unit/compute_resource_test.rb | 86 +++++++++±
>> test/unit/domain_parameter_test.rb | 3 +
>> 36 files changed, 546 insertions(+), 61 deletions(-)
>> rename app/controllers/{compute_resources/vms_controller.rb =>
>> compute_resources_vms_controller.rb} (91%)
>> delete mode 100644 app/views/compute_resources/vms/show.html.erb
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/form/_ec2.html.erb (100%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/form/_libvirt.html.erb (86%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/form/_ovirt.html.erb (91%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/form/libvirt/_network.html.erb (100%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/form/libvirt/_volume.html.erb (100%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/form/ovirt/_network.html.erb (100%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/form/ovirt/_volume.html.erb (100%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/index.html.erb (67%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/index/_ec2.html.erb (100%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/index/_libvirt.html.erb (81%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/index/_ovirt.html.erb (81%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/index/vmware.html.erb (100%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/new.html.erb (82%)
>> create mode 100644 app/views/compute_resources_vms/show.html.erb
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/show/libvirt.html.erb (100%)
>> rename app/views/{compute_resources/vms =>
>> compute_resources_vms}/show/ovirt.html.erb (100%)
>> create mode 100644
>> db/migrate/20120529113900_add_user_compute_resources.rb
>> create mode 100644
>> db/migrate/20120529115814_add_compute_resources_boolean_to_user.rb
>> create mode 100644
>> test/functional/compute_resources_vms_controller_test.rb
>>
>> diff --git a/app/controllers/application_controller.rb
>> b/app/controllers/application_controller.rb
>> index 7386253…fd022a9 100644
>> — a/app/controllers/application_controller.rb
>> +++ b/app/controllers/application_controller.rb
>> @@ -31,7 +31,7 @@ class ApplicationController < ActionController::Base
>> # Authorize the user for the requested action
>> def authorize(ctrl = params[:controller], action = params[:action])
>> return true if request.xhr?
>> - allowed = User.current.allowed_to?({:controller => ctrl.gsub(/::/,
>> "").underscore, :action => action})
>> + allowed = User.current.allowed_to?({:controller => ctrl.gsub(/::/,
>> "").gsub(///, "").underscore, :action => action})
>> allowed ? true : deny_access
>
> is this still required after renaming the host? does it hurt if we keep it?
>
I removed it and ran the tests: no it is no longer required.
It is only needed to make :controller => "ParentResources::ChildResource"
and :controller => "parent_resources/child_resource" work the same in
config/routes.rb.
The former is required for the website to work properly
(because of use_relative_controller! in the route set generator of
ActionDispatch),
whereas the latter is required for the tests to run properly for
nested resources,
and the two are unfortunately not compatible.
Note: I only saw the latter form on the rails tutorials.

Maybe you should discard that diff, as the only solution seems to be
not using nesting, and the Ruby::Namespace notation.
>>
>> end
>>
>> diff --git a/app/controllers/compute_resources_controller.rb
>> b/app/controllers/compute_resources_controller.rb
>> index 9ab5128…78b0577 100644
>> — a/app/controllers/compute_resources_controller.rb
>> +++ b/app/controllers/compute_resources_controller.rb
>> @@ -2,10 +2,16 @@ class ComputeResourcesController < ApplicationController
>> include Foreman::Controller::AutoCompleteSearch
>> AJAX_REQUESTS = %w{hardware_profile_selected cluster_selected}
>> before_filter :ajax_request, :only => AJAX_REQUESTS
>> - before_filter :find_by_id, :only => %w{show edit update destroy} +
>> AJAX_REQUESTS
>> + before_filter :find_by_id, :only => [:show, :edit, :update, :destroy] +
>> AJAX_REQUESTS
>
> did it not work? why did you change it? (just wondering)
>
Simply to be more uniform with the other files.
(and because I do not know what that %w{} magic does).
>>
>>
>> def index
>> - values = ComputeResource.search_for(params[:search], :order =>
>> params[:order])
>> + begin
>> + values =
>> ComputeResource.my_compute_resources.search_for(params[:search], :order =>
>> params[:order])
>> + rescue => e
>> + error e.to_s
>> + values = ComputeResource.my_compute_resources.search_for ""
>> + end
>> +
>> respond_to do |format|
>> format.html { @compute_resources = values.paginate :page =>
>> params[:page] }
>> format.json { render :json => values }
>> @@ -16,10 +22,19 @@ class ComputeResourcesController <
>> ApplicationController
>> @compute_resource = ComputeResource.new
>> end
>>
>> + def show
>> + respond_to do |format|
>> + format.html
>> + format.json { render :json => @compute_resource }
>> + end
>> + end
>> +
>> def create
>> if params[:compute_resource].present? &&
>> params[:compute_resource][:provider].present?
>> @compute_resource = ComputeResource.new_provider
>> params[:compute_resource]
>> if @compute_resource.save
>> + # Add the new compute resource to the user's filters
>> + @compute_resource.users << User.current
>> process_success :success_redirect => @compute_resource
>> else
>> process_error
>> @@ -31,9 +46,6 @@ class ComputeResourcesController < ApplicationController
>> end
>> end
>>
>> - def edit
>> - end
>
> why did you remove it? i think its a valid controller action?
>
This is like overriding a function without adding or changing
anything, isn't it?
However if it plays the role of a place holder to mean a valid action
one may want to override one day, then I guess you should keep it.

···

2012/5/29 Ohad Levy : > On Tue, May 29, 2012 at 1:24 PM, Olivier Favre wrote: >> >> - >> def update >> if @compute_resource.update_attributes(params[:compute_resource]) >> process_success >> @@ -82,5 +94,7 @@ class ComputeResourcesController < ApplicationController >> >> def find_by_id >> @compute_resource = ComputeResource.find(params[:id]) >> + not_found and return unless @compute_resource >> + deny_access and return unless >> ComputeResource.my_compute_resources.include?(@compute_resource) >> end >> end >> diff --git a/app/controllers/compute_resources/vms_controller.rb >> b/app/controllers/compute_resources_vms_controller.rb >> similarity index 91% >> rename from app/controllers/compute_resources/vms_controller.rb >> rename to app/controllers/compute_resources_vms_controller.rb >> index 370e9ba..20cd8df 100644 >> --- a/app/controllers/compute_resources/vms_controller.rb >> +++ b/app/controllers/compute_resources_vms_controller.rb >> @@ -1,4 +1,4 @@ >> -class ComputeResources::VmsController < ApplicationController >> +class ComputeResourcesVmsController < ApplicationController >> before_filter :find_compute_resource >> before_filter :find_vm, :only => [:show, :power, :console] >> >> @@ -61,6 +61,7 @@ class ComputeResources::VmsController < >> ApplicationController >> >> def find_compute_resource >> @compute_resource = ComputeResource.find(params[:compute_resource_id]) >> + @compute_resource = >> ComputeResource.my_compute_resources.find(params[:compute_resource_id]) >> rescue deny_access >> end >> >> def find_vm >> diff --git a/app/models/compute_resource.rb >> b/app/models/compute_resource.rb >> index 40a9ce8..18dc6b5 100644 >> --- a/app/models/compute_resource.rb >> +++ b/app/models/compute_resource.rb >> @@ -8,6 +8,7 @@ class ComputeResource < ActiveRecord::Base >> STI_PREFIX= "Foreman::Model" >> >> include Authorization >> + has_and_belongs_to_many :users, :join_table => "user_compute_resources" >> validates_format_of :name, :with => /\A(\S+)\Z/, :message => "can't be >> blank or contain white spaces." >> validates_uniqueness_of :name >> validates_presence_of :provider, :in => PROVIDERS >> @@ -17,9 +18,24 @@ class ComputeResource < ActiveRecord::Base >> has_many :hosts >> has_many :images, :dependent => :destroy >> >> + default_scope :order => 'LOWER(compute_resources.name)' >> + >> + scope :my_compute_resources, lambda { >> + user = User.current >> + if user.admin? >> + conditions = { } >> + else >> + conditions = sanitize_sql_for_conditions([" (compute_resources.id >> in (?))", user.compute_resources.map(&:id)]) >> + conditions.sub!(/\s*\(\)\s*/, "") >> + conditions.sub!(/^(?:\(\))?\s?(?:and|or)\s*/, "") >> + conditions.sub!(/\(\s*(?:or|and)\s*\(/, "((") >> + end >> + {:conditions => conditions} >> + } >> + >> # allows to create a specific compute class based on the provider. >> def self.new_provider args >> - raise "must provider a provider" unless provider = args[:provider] >> + raise "must provide a provider" unless provider = args[:provider] >> PROVIDERS.each do |p| >> return eval("#{STI_PREFIX}::#{p}").new(args) if p.downcase == >> provider.downcase >> end >> @@ -165,4 +181,22 @@ class ComputeResource < ActiveRecord::Base >> end.compact >> end >> >> + private >> + >> + def enforce_permissions operation >> + # We get called again with the operation being set to create >> + return true if operation == "edit" and new_record? >> + current = User.current >> + if current.allowed_to?("#{operation}_compute_resources".to_sym) >> + # If you can create compute resources then you can create them >> anywhere >> + return true if operation == "create" >> + # edit or delete >> + if current.allowed_to?("#{operation}_compute_resources".to_sym) >> + return true if >> ComputeResource.my_compute_resources(current).include? self >> + end >> + end >> + errors.add :base, "You do not have permission to #{operation} this >> domain" >> + false >> + end >> + >> end >> diff --git a/app/models/host.rb b/app/models/host.rb >> index 170a356..f343517 100644 >> --- a/app/models/host.rb >> +++ b/app/models/host.rb >> @@ -82,9 +82,10 @@ class Host < Puppet::Rails::Host >> user = User.current >> return { :conditions => "" } if user.admin? # Admin can see all hosts >> >> - owner_conditions = sanitize_sql_for_conditions(["((hosts.owner_id >> in (?) AND hosts.owner_type = 'Usergroup') OR (hosts.owner_id = ? AND >> hosts.owner_type = 'User'))", user.my_usergroups.map(&:id), user.id]) >> - domain_conditions = sanitize_sql_for_conditions([" >> (hosts.domain_id in (?))",dms = (user.domains).map(&:id)]) >> - hostgroup_conditions = sanitize_sql_for_conditions([" >> (hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)]) >> + owner_conditions = >> sanitize_sql_for_conditions(["((hosts.owner_id in (?) AND hosts.owner_type = >> 'Usergroup') OR (hosts.owner_id = ? AND hosts.owner_type = 'User'))", >> user.my_usergroups.map(&:id), user.id]) >> + domain_conditions = sanitize_sql_for_conditions([" >> (hosts.domain_id in (?))",dms = (user.domains).map(&:id)]) >> + compute_resource_conditions = sanitize_sql_for_conditions([" >> (hosts.compute_resource_id in (?))",(crs = >> user.compute_resources).map(&:id)]) >> + hostgroup_conditions = sanitize_sql_for_conditions([" >> (hosts.hostgroup_id in (?))",(hgs = user.hostgroups).map(&:id)]) >> >> fact_conditions = "" >> for user_fact in (ufs = user.user_facts) >> @@ -97,10 +98,11 @@ class Host < Puppet::Rails::Host >> >> conditions = "" >> if user.filtering? >> - conditions = "#{owner_conditions}" >> >> if user.filter_on_owner >> - (conditions = (user.domains_andor == "and") ? "(#{conditions}) >> and #{domain_conditions} " : "#{conditions} or #{domain_conditions} ") >> unless dms.empty? >> - (conditions = (user.hostgroups_andor == "and") ? "(#{conditions}) >> and #{hostgroup_conditions} " : "#{conditions} or #{hostgroup_conditions} ") >> unless hgs.empty? >> - (conditions = (user.facts_andor == "and") ? "(#{conditions}) >> and #{fact_conditions} " : "#{conditions} or #{fact_conditions} ") >> unless ufs.empty? >> + conditions = "#{owner_conditions}" >> >> if user.filter_on_owner >> + (conditions = (user.domains_andor == "and") ? >> "(#{conditions}) and #{domain_conditions} " : "#{conditions} or >> #{domain_conditions} ") unless dms.empty? >> + (conditions = (user.compute_resources_andor == "and") ? >> "(#{conditions}) and #{compute_resource_conditions} " : "#{conditions} or >> #{compute_resource_conditions} ") unless crs.empty? >> + (conditions = (user.hostgroups_andor == "and") ? >> "(#{conditions}) and #{hostgroup_conditions} " =A