[PATCH/foreman 1/1] Permissions patch to separate permssions on hosts from permissions for objects within hosts. This allows a user to be granted permission to edit the host (and so change the group or proxy) but not, for example, e

Development
1

Hi,

I've tried to apply it but it doesn't apply cleanly, any chance you
can rebase and fix it?

on a very quick glance, can you also change all debug messages
(logger.info) to logger.debug?

thanks,
Ohad

··· On Thu, Nov 24, 2011 at 4:02 PM, Greg Sutcliffe wrote: > From: Greg Sutcliffe > > This could probably be extended further if necessary. > > Refs #1324 > > Signed-off-by: Greg Sutcliffe > --- > app/models/host_parameter.rb | 9 ++++++++- > app/views/common_parameters/_parameter.erb | 14 +++++++++++--- > app/views/common_parameters/_parameters.erb | 2 +- > app/views/puppetclasses/_class_selection.html.erb | 12 +++++++++--- > app/views/puppetclasses/_classes.html.erb | 8 ++++++-- > lib/access_permissions.rb | 7 +++++++ > test/unit/host_parameter_test.rb | 12 ++++++------ > 7 files changed, 48 insertions(+), 16 deletions(-) > > diff --git a/app/models/host_parameter.rb b/app/models/host_parameter.rb > index 6ec0b4c..328c342 100644 > --- a/app/models/host_parameter.rb > +++ b/app/models/host_parameter.rb > @@ -12,6 +12,13 @@ class HostParameter < Parameter > # We get called again with the operation being set to create > return true if operation == "edit" and new_record? > > - self.host.enforce_permissions operation > + logger.info User.current.allowed_to?("#{operation}_params".to_sym).inspect > + logger.info operation.inspect > + if User.current.allowed_to?("#{operation}_params".to_sym) > + return true > + end > + > + return false > + # self.host.enforce_permissions operation > end > end > diff --git a/app/views/common_parameters/_parameter.erb b/app/views/common_parameters/_parameter.erb > index 99b419d..52de5a3 100644 > --- a/app/views/common_parameters/_parameter.erb > +++ b/app/views/common_parameters/_parameter.erb > @@ -2,14 +2,22 @@ >
> <%= f.label :name %> >
> - <%= f.text_field :name %> > + <% if authorized_for(:host_editing, :edit_params) -%> > + <%= f.text_field :name %> > + <% else -%> > + <%= f.text_field :name, :disabled => 'true' %> > + <% end -%> >
>
>
> <%= f.label :value %> >
> - <%= f.text_field :value, :class => "span10" %> > - <%= authorized_via_my_scope(params[:controller], params[:action]) ? link_to_remove_fields("remove", f) : "" %> > + <% if authorized_for(:host_editing, :edit_params) -%> > + <%= f.text_field :value, :class => "span10" %> > + <% else -%> > + <%= f.text_field :value, :class => "span10", :disabled => 'true' %> > + <% end -%> > + <%= authorized_for(:host_editing, :destroy_params) ? link_to_remove_fields("remove", f) : "" -%> >
>
> <%= f.hidden_field :nested %> > diff --git a/app/views/common_parameters/_parameters.erb b/app/views/common_parameters/_parameters.erb > index 21fad5e..0436af9 100644 > --- a/app/views/common_parameters/_parameters.erb > +++ b/app/views/common_parameters/_parameters.erb > @@ -2,5 +2,5 @@ > <% f.fields_for type do |builder| -%> > <%= render "common_parameters/parameter", :f => builder %> > <% end -%> > -

<%= authorized_via_my_scope(params[:controller], params[:action]) ? link_to_add_fields("+", f, type, "common_parameters/parameter") : "Add a parameter" %>

> +

<%= authorized_for(:host_editing, :create_params) ? link_to_add_fields("+", f, type, "common_parameters/parameter") : "" %>

> <% end -%> > diff --git a/app/views/puppetclasses/_class_selection.html.erb b/app/views/puppetclasses/_class_selection.html.erb > index 6937afb..e9e4a07 100644 > --- a/app/views/puppetclasses/_class_selection.html.erb > +++ b/app/views/puppetclasses/_class_selection.html.erb > @@ -4,9 +4,15 @@ > <%# hidden field to ensure that classes gets removed if none are defined -%> > <%= hidden_field_tag obj.class.to_s.downcase + "[puppetclass_ids][]" %> >
    > - <%= render :partial => "puppetclasses/selectedClasses", > - :collection => obj.puppetclasses ,:as => :klass, > - :locals => { :type => obj.class.to_s.downcase } %> > + <% if authorized_for(:host_editing, :edit_classes) -%> > + <%= render :partial => "puppetclasses/selectedClasses", > + :collection => obj.puppetclasses ,:as => :klass, > + :locals => { :type => obj.class.to_s.downcase } %> > + <% else -%> > + <% obj.puppetclasses.each do |klass| %> > +
  • <%= h klass.name %>
    • > + <% end -%> > + <% end -%> >
>
    > <% parent_classes(obj).each do |klass| %> > diff --git a/app/views/puppetclasses/_classes.html.erb b/app/views/puppetclasses/_classes.html.erb > index df5a77c..b5f0c27 100644 > --- a/app/views/puppetclasses/_classes.html.erb > +++ b/app/views/puppetclasses/_classes.html.erb > @@ -6,8 +6,12 @@ >
  • <%= link_to_function image_tag("bullet_toggle_plus.png") + " " + list.first, "$('#pc_#{list.first}').fadeToggle('slow')" %> >
      > <% for klass in list.last.sort -%> > - <% content_tag_for :li, klass, :title => "Click to add #{klass}", :class=> "#{cycle('even', 'odd')}" do %> > - <%= klass.name + link_to_add_puppetclass(klass, type) %> > + <% if not authorized_for(:host_editing, :edit_classes) -%> > +
    • <%= h klass.name %>
      • > + <% else -%> > + <% content_tag_for :li, klass, :title => "Click to add #{klass}", :class=> "#{cycle('even', 'odd')}" do %> > + <%= klass.name + link_to_add_puppetclass(klass, type) %> > + <% end -%> > <% end -%> > <% end -%> >
    > diff --git a/lib/access_permissions.rb b/lib/access_permissions.rb > index bf54b33..3f568e5 100644 > --- a/lib/access_permissions.rb > +++ b/lib/access_permissions.rb > @@ -77,6 +77,13 @@ Foreman::AccessControl.map do |map| > map.permission :build_hosts, {:hosts => [:setBuild, :cancelBuild]} > end > > + map.security_block :host_editing do |map| > + map.permission :edit_classes, {:host_editing => [:edit_classes]} > + map.permission :create_params, {:host_editing => [:create_params]} > + map.permission :edit_params, {:host_editing => [:edit_params]} > + map.permission :destroy_params, {:host_editing => [:destroy_params]} > + end > + > map.security_block :hypervisors do |map| > map.permission :view_hypervisors, {:hypervisors => [:index, :show]} > map.permission :create_hypervisors, {:hypervisors => [:new, :create]} > diff --git a/test/unit/host_parameter_test.rb b/test/unit/host_parameter_test.rb > index 4da97f1..71dea8b 100644 > --- a/test/unit/host_parameter_test.rb > +++ b/test/unit/host_parameter_test.rb > @@ -35,11 +35,11 @@ class HostParameterTest < ActiveSupport::TestCase > assert @parameter2.valid? > end > > - def setup_user operation > + def setup_user operation, type = "hosts" > @one = users(:one) > as_admin do > - role = Role.find_or_create_by_name :name => "#{operation}_hosts" > - role.permissions = ["#{operation}_hosts".to_sym] > + role = Role.find_or_create_by_name :name => "#{operation}_#{type}" > + role.permissions = ["#{operation}_#{type}".to_sym] > @one.roles = [role] > @one.domains = [] > @one.hostgroups = [] > @@ -73,7 +73,7 @@ class HostParameterTest < ActiveSupport::TestCase > end > > test "user with create permissions should be able to create when unconstrained" do > - setup_user "create" > + setup_user "create", "params" > as_admin do > @one.domains = [] > end > @@ -90,7 +90,7 @@ class HostParameterTest < ActiveSupport::TestCase > end > > test "user with destroy permissions should be able to destroy" do > - setup_user "destroy" > + setup_user "destroy", "params" > record = HostParameter.first > assert record.destroy > assert record.frozen? > @@ -104,7 +104,7 @@ class HostParameterTest < ActiveSupport::TestCase > end > > test "user with edit permissions should be able to edit" do > - setup_user "edit" > + setup_user "edit", "params" > record = HostParameter.first > record.name = "renamed" > assert record.save > -- > 1.7.7.4 >
2

Not a problem, I'll dig out the patch and get it onto a clean tree as
soon as I can. ETA a day or two.

Greg

··· On 13 December 2011 12:44, Ohad Levy wrote: > Hi, > > I've tried to apply it but it doesn't apply cleanly, any chance you > can rebase and fix it? > > on a very quick glance, can you also change all debug messages > (logger.info) to logger.debug? > > thanks, > Ohad