[Ovirt-devel] [PATCH] more permissions validation for non-privileged users.
Jason Guiditta
jguiditt at redhat.com
Mon Jan 26 21:56:25 UTC 2009
Overall ACK, there are two minor things I think should be fixed, and a
couple thoughts inline. However, any of them could be addressed in a
follow-up patch if that works better.
-j
On Thu, 2009-01-22 at 23:01 +0000, Scott Seago wrote:
> Fixed some of the error handling for popup and json ajax responses, and hid action links from non-privileged users where those actions were not appropriate for them.
>
> Signed-off-by: Scott Seago <sseago at redhat.com>
> ---
> src/app/controllers/application.rb | 40 +++++++++++++--------
> src/app/controllers/dashboard_controller.rb | 1 +
> src/app/controllers/hardware_controller.rb | 5 +--
> src/app/controllers/host_controller.rb | 9 +++++
> src/app/controllers/network_controller.rb | 25 ++++++++-----
> src/app/controllers/pool_controller.rb | 8 ++--
> src/app/controllers/quota_controller.rb | 3 --
> src/app/controllers/resources_controller.rb | 2 -
> src/app/controllers/smart_pools_controller.rb | 3 +-
> src/app/controllers/storage_controller.rb | 25 ++++---------
> src/app/controllers/vm_controller.rb | 3 --
> src/app/models/smart_pool.rb | 8 +++--
> src/app/views/hardware/show_hosts.rhtml | 24 +++++++-----
> src/app/views/hardware/show_storage.rhtml | 28 +++++++++------
> src/app/views/hardware/show_vms.rhtml | 14 +++++--
> src/app/views/layouts/_side_toolbar.rhtml | 4 +-
> src/app/views/layouts/_tree.rhtml | 9 +++--
> src/app/views/layouts/popup-error.rhtml | 5 +++
> src/app/views/resources/show_vms.rhtml | 46 ++++++++++++++----------
> src/app/views/user/_grid.rhtml | 11 ++++--
> src/app/views/user/_show.rhtml | 9 +++--
> src/public/stylesheets/ovirt-tree/tree.css | 8 ++---
> 22 files changed, 167 insertions(+), 123 deletions(-)
> create mode 100644 src/app/views/layouts/popup-error.rhtml
>
> diff --git a/src/app/controllers/application.rb b/src/app/controllers/application.rb
> index 3f75979..1c3f99e 100644
> --- a/src/app/controllers/application.rb
> +++ b/src/app/controllers/application.rb
> @@ -82,26 +82,36 @@ class ApplicationController < ActionController::Base
> def pre_show
> end
>
> - def authorize_user
> - authorize_action(false)
> + def authorize_user(msg=nil)
> + authorize_action(false,msg)
> end
> - def authorize_admin
> - authorize_action(true)
> + def authorize_admin(msg=nil)
> + authorize_action(true,msg)
> end
> - def authorize_action(is_modify_action)
> + def authorize_action(is_modify_action, msg=nil)
> + msg ||= 'You do not have permission to create or modify this item '
> if @perm_obj
> set_perms(@perm_obj)
> unless (is_modify_action ? @can_modify : @can_control_vms)
> - @redir_obj = @perm_obj unless @redir_obj
> - flash[:notice] = 'You do not have permission to create or modify this item '
> - if @json_hash
> - @json_hash[:success] = false
> - @json_hash[:alert] = flash[:notice]
> - render :json => @json_hash
> - elsif @redir_controller
> - redirect_to :controller => @redir_controller, :action => 'show', :id => @redir_obj
> - else
> - redirect_to :action => 'show', :id => @redir_obj
> + respond_to do |format|
> + format.html do
> + @title = "Access denied"
> + @errmsg = msg
> + if params[:ajax]
> + render :template => 'layouts/popup-error', :layout => 'tabs-and-content'
> + elsif params[:nolayout]
> + render :template => 'layouts/popup-error', :layout => 'help-and-content'
> + else
> + render :template => 'layouts/popup-error', :layout => 'popup'
> + end
> + end
> + format.json do
> + @json_hash ||= {}
> + @json_hash[:success] = false
> + @json_hash[:alert] = msg
> + render :json => @json_hash
> + end
> + format.xml { head :forbidden }
> end
> false
> end
> diff --git a/src/app/controllers/dashboard_controller.rb b/src/app/controllers/dashboard_controller.rb
> index 00398a5..821fb3f 100644
> --- a/src/app/controllers/dashboard_controller.rb
> +++ b/src/app/controllers/dashboard_controller.rb
> @@ -29,6 +29,7 @@ class DashboardController < ApplicationController
>
> def index
> @task_types = Task::TASK_TYPES_OPTIONS
> + @user = get_login_user
> show_tasks
> end
>
> diff --git a/src/app/controllers/hardware_controller.rb b/src/app/controllers/hardware_controller.rb
> index 5c14eec..fc16a27 100644
> --- a/src/app/controllers/hardware_controller.rb
> +++ b/src/app/controllers/hardware_controller.rb
> @@ -29,7 +29,8 @@ class HardwareController < PoolController
>
> before_filter :pre_modify, :only => [:add_hosts, :move_hosts,
> :add_storage, :move_storage,
> - :create_storage, :delete_storage]
> + :create_storage, :delete_storage,
> + :move, :removestorage]
>
> def index
> if params[:path]
> @@ -174,7 +175,6 @@ class HardwareController < PoolController
> end
>
> def move
> - pre_modify
> @resource_type = params[:resource_type]
> @id = params[:id]
> @pools = HardwarePool.get_default_pool.full_set_nested(:method => :json_hash_element,
> @@ -330,7 +330,6 @@ class HardwareController < PoolController
> end
>
> def removestorage
> - pre_modify
> render :layout => 'popup'
> end
>
> diff --git a/src/app/controllers/host_controller.rb b/src/app/controllers/host_controller.rb
> index da630f7..02ad8c9 100644
> --- a/src/app/controllers/host_controller.rb
> +++ b/src/app/controllers/host_controller.rb
> @@ -31,6 +31,7 @@ class HostController < ApplicationController
> end
>
> before_filter :pre_action, :only => [:host_action, :enable, :disable, :clear_vms, :edit_network]
> + before_filter :pre_addhost, :only => [:addhost]
>
> # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
> verify :method => [:post, :put], :only => [ :create, :update ],
> @@ -85,6 +86,14 @@ class HostController < ApplicationController
> render :layout => 'popup'
> end
>
> + def pre_addhost
> + @pool = Pool.find(params[:hardware_pool_id])
> + @parent = @pool.parent
> + @perm_obj = @pool
> + @current_pool_id=@pool.id
> + authorize_admin
> + end
> +
> def add_to_smart_pool
> @pool = SmartPool.find(params[:smart_pool_id])
> render :layout => 'popup'
> diff --git a/src/app/controllers/network_controller.rb b/src/app/controllers/network_controller.rb
> index e4faf7b..7328e66 100644
> --- a/src/app/controllers/network_controller.rb
> +++ b/src/app/controllers/network_controller.rb
> @@ -20,22 +20,24 @@
> class NetworkController < ApplicationController
> ########################## Networks related actions
>
> - def network_permissions
> + before_filter :pre_list, :only => [:list]
> +
> + def authorize_admin
> # TODO more robust permission system
> # either by subclassing network from pool
> # or by extending permission model to accomodate
> # any object
> @default_pool = HardwarePool.get_default_pool
> - set_perms(@default_pool)
> - unless @can_modify
> - flash[:notice] = 'You do not have permission to view networks'
> - redirect_to :controller => 'dashboard'
> - end
> + @perm_obj=@default_pool
> + super('You do not have permission to access networks')
> end
>
> - def list
> + def pre_list
> @networks = Network.find(:all)
> - network_permissions
> + authorize_admin
> + end
> +
> + def list
> respond_to do |format|
> format.html {
> render :layout => 'tabs-and-content' if params[:ajax]
> @@ -51,9 +53,12 @@ class NetworkController < ApplicationController
> json_list(Network.find(:all), [:id, :name, :type, [:boot_type, :label]])
> end
>
> + def pre_show
> + @network = Network.find(params[:id])
> + authorize_admin
> + end
> +
> def show
> - @network = Network.find(params[:id])
> - network_permissions
> respond_to do |format|
> format.html { render :layout => 'selection' }
> format.xml { render :xml => @network.to_xml }
> diff --git a/src/app/controllers/pool_controller.rb b/src/app/controllers/pool_controller.rb
> index b8d0f10..2809d6d 100644
> --- a/src/app/controllers/pool_controller.rb
> +++ b/src/app/controllers/pool_controller.rb
> @@ -62,8 +62,10 @@ class PoolController < ApplicationController
> end
>
> def users_json
> - json_list(@pool.permissions,
> - [:grid_id, :uid, :user_role, :source])
> + attr_list = []
> + attr_list << :grid_id if params[:checkboxes]
> + attr_list += [:uid, :user_role, :source]
> + json_list(@pool.permissions, attr_list)
> end
>
> def hosts_json(args)
> @@ -103,7 +105,6 @@ class PoolController < ApplicationController
> def pre_new
> @parent = Pool.find(params[:parent_id])
> @perm_obj = @parent
> - @redir_controller = @perm_obj.get_controller
> @current_pool_id=@parent.id
> end
> def pre_create
> @@ -114,7 +115,6 @@ class PoolController < ApplicationController
> @parent = Pool.find(params[:parent_id])
> end
> @perm_obj = @parent
> - @redir_controller = @perm_obj.get_controller
> @current_pool_id=@parent.id
> end
> def pre_show_pool
> diff --git a/src/app/controllers/quota_controller.rb b/src/app/controllers/quota_controller.rb
> index 58446d4..17fdc20 100644
> --- a/src/app/controllers/quota_controller.rb
> +++ b/src/app/controllers/quota_controller.rb
> @@ -84,12 +84,10 @@ class QuotaController < ApplicationController
> def pre_new
> @quota = Quota.new( { :pool_id => params[:pool_id]})
> @perm_obj = @quota.pool
> - @redir_controller = @perm_obj.get_controller
> end
> def pre_create
> @quota = Quota.new(params[:quota])
> @perm_obj = @quota.pool
> - @redir_controller = @perm_obj.get_controller
> end
> def pre_show
> @quota = Quota.find(params[:id])
> @@ -98,7 +96,6 @@ class QuotaController < ApplicationController
> def pre_edit
> @quota = Quota.find(params[:id])
> @perm_obj = @quota.pool
> - @redir_controller = @perm_obj.get_controller
> end
>
> end
> diff --git a/src/app/controllers/resources_controller.rb b/src/app/controllers/resources_controller.rb
> index 3c6e3ee..7bed533 100644
> --- a/src/app/controllers/resources_controller.rb
> +++ b/src/app/controllers/resources_controller.rb
> @@ -167,7 +167,6 @@ class ResourcesController < PoolController
> @pool = VmResourcePool.find(params[:id])
> @parent = @pool.parent
> @perm_obj = @pool.parent
> - @redir_obj = @pool
> @current_pool_id=@pool.id
> end
> def pre_show
> @@ -179,7 +178,6 @@ class ResourcesController < PoolController
> @pool = VmResourcePool.find(params[:id])
> @parent = @pool.parent
> @perm_obj = @pool
> - @redir_obj = @pool
> authorize_user
> end
>
> diff --git a/src/app/controllers/smart_pools_controller.rb b/src/app/controllers/smart_pools_controller.rb
> index 10dbf9a..cbfbd1c 100644
> --- a/src/app/controllers/smart_pools_controller.rb
> +++ b/src/app/controllers/smart_pools_controller.rb
> @@ -24,7 +24,7 @@ class SmartPoolsController < PoolController
> :add_storage, :remove_storage,
> :add_vms, :remove_vms,
> :add_pools, :remove_pools,
> - :add_items]
> + :add_items, :add_pool_dialog]
> def show_vms
> show
> end
> @@ -65,7 +65,6 @@ class SmartPoolsController < PoolController
> end
>
> def add_pool_dialog
> - pre_modify
> @selected_pools = @pool.tagged_pools.collect {|pool| pool.id}
> render :layout => 'popup'
> end
> diff --git a/src/app/controllers/storage_controller.rb b/src/app/controllers/storage_controller.rb
> index e4b72f1..2b76f44 100644
> --- a/src/app/controllers/storage_controller.rb
> +++ b/src/app/controllers/storage_controller.rb
> @@ -26,6 +26,7 @@ class StorageController < ApplicationController
> before_filter :pre_new2, :only => [:new2]
> before_filter :pre_json, :only => [:storage_volumes_json]
> before_filter :pre_create_volume, :only => [:create_volume]
> + before_filter :pre_add, :only => [:add, :addstorage]
>
> def index
> list
> @@ -258,27 +259,15 @@ class StorageController < ApplicationController
> end
> end
>
> - def add_internal
> - @hardware_pool = HardwarePool.find(params[:hardware_pool_id])
> - @perm_obj = @hardware_pool
> - @redir_controller = @perm_obj.get_controller
> - authorize_admin
> - @storage_pools = @hardware_pool.storage_volumes
> - @storage_types = StoragePool::STORAGE_TYPE_PICKLIST
> - end
> -
Add storage does not render the error popup if I revoke privileges once
the form has rendered, but before submit. I think this is because the
javascript form handler does not have anything set of on error, only
success, so we might need a generic error function that we can call, as
this is sure to come up elsewhere.
> def addstorage
> - add_internal
> render :layout => 'popup'
> end
>
> def add
> - add_internal
> render :layout => false
> end
>
> def new
> - add_internal
> render :layout => false
> end
Is this going to replace 'new2'? That one kinda bugs me.
>
> @@ -396,7 +385,13 @@ class StorageController < ApplicationController
> def pre_new
> @hardware_pool = HardwarePool.find(params[:hardware_pool_id])
> @perm_obj = @hardware_pool
> - @redir_controller = @perm_obj.get_controller
> + authorize_admin
> + @storage_pools = @hardware_pool.storage_volumes
> + @storage_types = StoragePool::STORAGE_TYPE_PICKLIST
> + end
> +
> + def pre_add
> + pre_new
> end
pre_new is only ever called by pre_add, why not just make it one method,
like maybe 'pre_modify', or even just pre_add?
>
> def pre_new2
> @@ -406,7 +401,6 @@ class StorageController < ApplicationController
> end
> @storage_pool = StoragePool.factory(params[:storage_type], new_params)
> @perm_obj = @storage_pool.hardware_pool
> - @redir_controller = @storage_pool.hardware_pool.get_controller
> authorize_admin
> end
> def pre_create
> @@ -416,12 +410,10 @@ class StorageController < ApplicationController
> end
> @storage_pool = StoragePool.factory(type, pool)
> @perm_obj = @storage_pool.hardware_pool
> - @redir_controller = @storage_pool.hardware_pool.get_controller
> end
> def pre_edit
> @storage_pool = StoragePool.find(params[:id])
> @perm_obj = @storage_pool.hardware_pool
> - @redir_obj = @storage_pool
> end
> def pre_create_volume
> volume = params[:storage_volume]
> @@ -430,7 +422,6 @@ class StorageController < ApplicationController
> end
> @storage_volume = StorageVolume.factory(type, volume)
> @perm_obj = @storage_volume.storage_pool.hardware_pool
> - @redir_controller = @storage_volume.storage_pool.hardware_pool.get_controller
> authorize_admin
> end
> def pre_json
> diff --git a/src/app/controllers/vm_controller.rb b/src/app/controllers/vm_controller.rb
> index 701dea8..56501fd 100644
> --- a/src/app/controllers/vm_controller.rb
> +++ b/src/app/controllers/vm_controller.rb
> @@ -332,7 +332,6 @@ class VmController < ApplicationController
> @vm.vm_resource_pool = @vm_resource_pool
> end
> @perm_obj = @vm.vm_resource_pool
> - @redir_controller = 'resources'
> @current_pool_id=@perm_obj.id
> _setup_provisioning_options
> end
> @@ -348,7 +347,6 @@ class VmController < ApplicationController
> end
> @vm = Vm.new(params[:vm])
> @perm_obj = @vm.vm_resource_pool
> - @redir_controller = 'resources'
> @current_pool_id=@perm_obj.id
> end
> def pre_show
> @@ -359,7 +357,6 @@ class VmController < ApplicationController
> def pre_edit
> @vm = Vm.find(params[:id])
> @perm_obj = @vm.vm_resource_pool
> - @redir_obj = @vm
> @current_pool_id=@perm_obj.id
> _setup_provisioning_options
> end
> diff --git a/src/app/models/smart_pool.rb b/src/app/models/smart_pool.rb
> index 772ffef..7df26fa 100644
> --- a/src/app/models/smart_pool.rb
> +++ b/src/app/models/smart_pool.rb
> @@ -87,9 +87,11 @@ class SmartPool < Pool
> user_pools <<[child_pool.name, child_pool.id]
> end
> else
> - pool_element[:children].each do |child_element|
> - child_pool = child_element[:obj]
> - other_pools << [pool.name + " > " + child_pool.name, child_pool.id]
> + if pool_element.has_key?(:children)
> + pool_element[:children].each do |child_element|
> + child_pool = child_element[:obj]
> + other_pools << [pool.name + " > " + child_pool.name, child_pool.id]
> + end
> end
> end
> end
> diff --git a/src/app/views/hardware/show_hosts.rhtml b/src/app/views/hardware/show_hosts.rhtml
> index 2fd29bc..64e5d91 100644
> --- a/src/app/views/hardware/show_hosts.rhtml
> +++ b/src/app/views/hardware/show_hosts.rhtml
> @@ -1,11 +1,13 @@
> <div id="toolbar_nav">
> <ul>
> - <li><a href="<%= url_for :controller => 'host', :action => 'addhost', :hardware_pool_id => @pool %>" rel="facebox[.bolder]"><%= image_tag "icon_addhost.png", :style=>"vertical-align:middle;" %> Add Host</a></li>
> - <li>
> - <a id="move_link" href="#" onClick="return validate_for_move();"><%= image_tag "icon_move.png", :style=>"vertical-align:middle;" %> Move</a>
> - <a id="move_link_hidden" href="<%= url_for :controller => 'hardware', :action => 'move', :id => @pool, :resource_type=>'hosts' %>" rel="facebox[.bolder]" style="display:none" ></a>
> - </li>
> - <li>
> + <%if @can_modify -%>
> + <li><a href="<%= url_for :controller => 'host', :action => 'addhost', :hardware_pool_id => @pool %>" rel="facebox[.bolder]"><%= image_tag "icon_addhost.png", :style=>"vertical-align:middle;" %> Add Host</a></li>
> + <li>
> + <a id="move_link" href="#" onClick="return validate_for_move();"><%= image_tag "icon_move.png", :style=>"vertical-align:middle;" %> Move</a>
> + <a id="move_link_hidden" href="<%= url_for :controller => 'hardware', :action => 'move', :id => @pool, :resource_type=>'hosts' %>" rel="facebox[.bolder]" style="display:none" ></a>
> + </li>
> + <% end -%>
> + <li>
> <%= image_tag "icon_smartpool.png", :style => "vertical-align:middle;" %> Add to Smart Pool <%= image_tag "icon_toolbar_arrow.gif", :style => "vertical-align:middle;" %>
> <ul>
> <% smart_pools = SmartPool.smart_pools_for_user(@user) %>
> @@ -19,8 +21,8 @@
> </li>
> <% } %>
> </ul>
> - </li>
> - <% if @pool.id != HardwarePool.get_default_pool.id %>
> + </li>
> + <% if @can_modify and (@pool.id != HardwarePool.get_default_pool.id) %>
> <li><a href="#" onClick="remove_hosts()"><%= image_tag "icon_remove.png" %> Remove</a></li>
> <% end %>
> </ul>
> @@ -111,8 +113,10 @@
>
> <div class="no-grid-items-text">
> No hosts found in this pool. <br/><br/>
> - <%= image_tag "icon_addhost.png", :style=>"vertical-align:middle;" %>
> - <a href="<%= url_for :controller => 'host', :action => 'addhost', :hardware_pool_id => @pool %>" rel="facebox[.bolder]">Add first host to this hardware pool</a>
> + <%if @can_modify -%>
> + <%= image_tag "icon_addhost.png", :style=>"vertical-align:middle;" %>
> + <a href="<%= url_for :controller => 'host', :action => 'addhost', :hardware_pool_id => @pool %>" rel="facebox[.bolder]">Add first host to this hardware pool</a>
> + <% end -%>
> </div>
> </div>
> </div>
> diff --git a/src/app/views/hardware/show_storage.rhtml b/src/app/views/hardware/show_storage.rhtml
> index 5643c83..5180be6 100644
> --- a/src/app/views/hardware/show_storage.rhtml
> +++ b/src/app/views/hardware/show_storage.rhtml
> @@ -1,10 +1,12 @@
> <div id="toolbar_nav">
> <ul>
> - <li><a href="<%= url_for :controller => 'storage', :action => 'addstorage', :hardware_pool_id => @pool %>" rel="facebox[.bolder]"><%= image_tag "icon_addstorage.png", :style => "vertical-align:middle;" %> Add Storage Server</a></li>
> - <li>
> - <a href="#" onClick="return validate_storage_for_move();" ><%= image_tag "icon_move.png", :style=>"vertical-align:middle;" %> Move</a>
> - <a id="move_link_hidden" href="<%= url_for :controller => 'hardware', :action => 'move', :id => @pool, :resource_type=>'storage' %>" rel="facebox[.bolder]" style="display:none" ></a>
> - </li>
> + <%if @can_modify -%>
> + <li><a href="<%= url_for :controller => 'storage', :action => 'addstorage', :hardware_pool_id => @pool %>" rel="facebox[.bolder]"><%= image_tag "icon_addstorage.png", :style => "vertical-align:middle;" %> Add Storage Server</a></li>
> + <li>
> + <a href="#" onClick="return validate_storage_for_move();" ><%= image_tag "icon_move.png", :style=>"vertical-align:middle;" %> Move</a>
> + <a id="move_link_hidden" href="<%= url_for :controller => 'hardware', :action => 'move', :id => @pool, :resource_type=>'storage' %>" rel="facebox[.bolder]" style="display:none" ></a>
> + </li>
> + <% end -%>
> <li>
> <%= image_tag "icon_smartpool.png", :style => "vertical-align:middle;" %> Add to Smart Pool <%= image_tag "icon_toolbar_arrow.gif", :style => "vertical-align:middle;" %>
> <ul>
> @@ -20,10 +22,12 @@
> <% } %>
> </ul>
> </li>
> - <li>
> - <a href="#" onClick="return validate_storage_for_remove();" ><%= image_tag "icon_remove.png", :style=>"vertical-align:middle;" %> Remove</a>
> - <a id="remove_link_hidden" href="<%= url_for :controller => 'hardware', :action => 'removestorage', :id => @pool %>" rel="facebox[.bolder]" style="display:none" ></a>
> - </li>
> + <%if @can_modify -%>
> + <li>
> + <a href="#" onClick="return validate_storage_for_remove();" ><%= image_tag "icon_remove.png", :style=>"vertical-align:middle;" %> Remove</a>
> + <a id="remove_link_hidden" href="<%= url_for :controller => 'hardware', :action => 'removestorage', :id => @pool %>" rel="facebox[.bolder]" style="display:none" ></a>
> + </li>
> + <% end -%>
> </ul>
> </div>
>
> @@ -141,8 +145,10 @@ ${htmlList(pools)}
>
> <div class="no-grid-items-text">
> No storage Volumes found in this pool. <br/><br/>
> - <%= image_tag "icon_addhost.png", :style=>"vertical-align:middle;" %>
> - <a href="<%= url_for :controller => 'storage', :action => 'addstorage', :hardware_pool_id => @pool %>" rel="facebox[.bolder]">Add first storage volume to this hardware pool</a>
> + <%if @can_modify -%>
> + <%= image_tag "icon_addhost.png", :style=>"vertical-align:middle;" %>
> + <a href="<%= url_for :controller => 'storage', :action => 'addstorage', :hardware_pool_id => @pool %>" rel="facebox[.bolder]">Add first storage volume to this hardware pool</a>
> + <% end -%>
> </div>
> </div>
> </div>
> diff --git a/src/app/views/hardware/show_vms.rhtml b/src/app/views/hardware/show_vms.rhtml
> index 6a8ded5..a829611 100644
> --- a/src/app/views/hardware/show_vms.rhtml
> +++ b/src/app/views/hardware/show_vms.rhtml
> @@ -1,6 +1,8 @@
> <div id="toolbar_nav">
> <ul>
> - <li><a href="<%= url_for :controller => 'resources', :action => 'new', :parent_id => @pool %>" rel="facebox[.bolder]"><%= image_tag "icon_add_vmpool.png", :style => "vertical-align:middle;" %> New Virtual Machine Pool</a></li>
> + <%if @can_modify -%>
> + <li><a href="<%= url_for :controller => 'resources', :action => 'new', :parent_id => @pool %>" rel="facebox[.bolder]"><%= image_tag "icon_add_vmpool.png", :style => "vertical-align:middle;" %> New Virtual Machine Pool</a></li>
> + <% end -%>
> <li>
> <%= image_tag "icon_smartpool.png", :style => "vertical-align:middle;" %> Add to Smart Pool <%= image_tag "icon_toolbar_arrow.gif", :style => "vertical-align:middle;" %>
> <ul>
> @@ -16,7 +18,9 @@
> <% } %>
> </ul>
> </li>
> - <li><a href="#" onClick="delete_vm_pools()"><%= image_tag "icon_delete_white.png", :style => "vertical-align:middle;" %> Delete</a></li>
> + <%if @can_modify -%>
> + <li><a href="#" onClick="delete_vm_pools()"><%= image_tag "icon_delete_white.png", :style => "vertical-align:middle;" %> Delete</a></li>
> + <% end -%>
> </ul>
> </div>
> <script type="text/javascript">
> @@ -92,8 +96,10 @@
>
> <div class="no-grid-items-text">
> No VM Resource Pools found in this hardware pool. <br/><br/>
> - <%= image_tag "icon_add_vmpool.png", :style => "vertical-align:middle;" %>
> - <a href="<%= url_for :controller => 'resources', :action => 'new', :parent_id => @pool %>" rel="facebox[.bolder]">Add first vm resource pool to this hardware pool</a></li>
> + <%if @can_modify -%>
> + <%= image_tag "icon_add_vmpool.png", :style => "vertical-align:middle;" %>
> + <a href="<%= url_for :controller => 'resources', :action => 'new', :parent_id => @pool %>" rel="facebox[.bolder]">Add first vm resource pool to this hardware pool</a></li>
> + <% end -%>
> </div>
> </div>
> </div>
> diff --git a/src/app/views/layouts/_side_toolbar.rhtml b/src/app/views/layouts/_side_toolbar.rhtml
> index 4b92bcf..bc52ea3 100644
> --- a/src/app/views/layouts/_side_toolbar.rhtml
> +++ b/src/app/views/layouts/_side_toolbar.rhtml
> @@ -10,7 +10,7 @@
> end %>
>
> <%if pool -%>
> - <%if pool[:type]=="HardwarePool" -%>
> + <%if pool[:type]=="HardwarePool" and @can_modify -%>
> <div class="toolbar" style="float:left;">
> <a href="<%= url_for :controller => :hardware, :action => 'new', :parent_id => pool %>" rel="facebox[.bolder]">
> <%=image_tag "icon_add_hardwarepool.png", :title=>"Add Hardware Pool" %>
> @@ -28,7 +28,7 @@
> <%=image_tag "icon_add_smartpool.png", :title=>"Add Smart Pool" %>
> </a>
> </div>
> -<%if pool -%>
> +<%if pool and @can_modify -%>
> <div class="toolbar" style="float:left;">
> <a href="#conf_nav_delete_pool" rel="facebox[.bolder]">
> <%= image_tag "icon_delete.gif", :title=>"Delete Selected Pool" %>
> diff --git a/src/app/views/layouts/_tree.rhtml b/src/app/views/layouts/_tree.rhtml
> index fa3effc..350908c 100644
> --- a/src/app/views/layouts/_tree.rhtml
> +++ b/src/app/views/layouts/_tree.rhtml
> @@ -103,9 +103,12 @@
> <%= link_to "Dashboard", dashboard_url, { :id => "dashboard"} %>
> </div>
> <% network_selected = "current" if controller.controller_name == "network" %>
> -<div class="nav-networks <%= network_selected %>">
> - <%= link_to "Networks", {:controller => "network", :action => "list", :ajax => true}, { :id => "networks"} %>
> -</div>
> +
> +<%if HardwarePool.get_default_pool.can_modify(@user) -%>
> + <div class="nav-networks <%= network_selected %>">
> + <%= link_to "Networks", {:controller => "network", :action => "list", :ajax => true}, { :id => "networks"} %>
> + </div>
> +<% end -%>
> <form id="nav_tree_form">
> <div class="nav-tree">
> <ul id="nav_tree" class="ovirt-tree"></ul>
> diff --git a/src/app/views/layouts/popup-error.rhtml b/src/app/views/layouts/popup-error.rhtml
> new file mode 100644
> index 0000000..5fadf76
> --- /dev/null
> +++ b/src/app/views/layouts/popup-error.rhtml
> @@ -0,0 +1,5 @@
> +<%- content_for :title do -%>
> + <%= @title %>
> +<%- end -%>
> +<%= @errmsg %>
> +<%= ok_footer %>
As I mentioned in irc, maybe this ok_footer gets a check for the :ajax
param, otherwise, there is at least on case I hit where it displays in
the main content area as regular content, so this button doesn't do
anything.
> diff --git a/src/app/views/resources/show_vms.rhtml b/src/app/views/resources/show_vms.rhtml
> index 6f757f9..1e75d35 100644
> --- a/src/app/views/resources/show_vms.rhtml
> +++ b/src/app/views/resources/show_vms.rhtml
> @@ -1,21 +1,25 @@
> <div id="toolbar_nav">
> <ul>
> - <li><a href="<%= url_for :controller => 'vm', :action => 'new', :vm_resource_pool_id => @pool %>" rel="facebox[.bolder]"><%= image_tag "icon_addhost.png", :style => "vertical-align:middle;" %> Add Virtual Machine</a></li>
> - <li>
> - <%= image_tag "icon_move.png", :style => "vertical-align:middle;" %> Actions <%= image_tag "icon_toolbar_arrow.gif", :style => "vertical-align:middle;" %>
> - <ul>
> - <% @actions.each_index { |index| %>
> - <li onClick="vm_actions('<%=@actions[index][1]%>')"
> - <% if (index == @actions.length - 1) or @actions[index].length == 4 %>
> - style="border-bottom: 1px solid #CCCCCC;"
> - <% end %>
> - >
> - <%= image_tag @actions[index][2]%>
> - <%=@actions[index][0]%>
> - </li>
> - <% } %>
> - </ul>
> - </li>
> + <%if @can_modify -%>
> + <li><a href="<%= url_for :controller => 'vm', :action => 'new', :vm_resource_pool_id => @pool %>" rel="facebox[.bolder]"><%= image_tag "icon_addhost.png", :style => "vertical-align:middle;" %> Add Virtual Machine</a></li>
> + <% end -%>
> + <%if @can_control_vms and -%>
> + <li>
> + <%= image_tag "icon_move.png", :style => "vertical-align:middle;" %> Actions <%= image_tag "icon_toolbar_arrow.gif", :style => "vertical-align:middle;" %>
> + <ul>
> + <% @actions.each_index { |index| %>
> + <li onClick="vm_actions('<%=@actions[index][1]%>')"
> + <% if (index == @actions.length - 1) or @actions[index].length == 4 %>
> + style="border-bottom: 1px solid #CCCCCC;"
> + <% end %>
> + >
> + <%= image_tag @actions[index][2]%>
> + <%=@actions[index][0]%>
> + </li>
> + <% } %>
> + </ul>
> + </li>
> + <% end -%>
> <li>
> <%= image_tag "icon_smartpool.png", :style => "vertical-align:middle;" %> Add to Smart Pool <%= image_tag "icon_toolbar_arrow.gif", :style => "vertical-align:middle;" %>
> <ul>
> @@ -31,7 +35,9 @@
> <% } %>
> </ul>
> </li>
> - <li><a href="#" onClick="delete_vms()"><%= image_tag "icon_delete_white.png", :style => "vertical-align:middle;" %> Delete</a></li>
> + <%if @can_modify -%>
> + <li><a href="#" onClick="delete_vms()"><%= image_tag "icon_delete_white.png", :style => "vertical-align:middle;" %> Delete</a></li>
> + <% end -%>
> </ul>
> </div>
> <script type="text/javascript">
> @@ -118,8 +124,10 @@
>
> <div class="no-grid-items-text">
> No vms found in this pool. <br/><br/>
> - <%= image_tag "icon_addhost.png", :style => "vertical-align:middle;" %>
> - <a href="<%= url_for :controller => 'vm', :action => 'new', :vm_resource_pool_id => @pool %>" rel="facebox[.bolder]">Add first virtual machine to resource pool</a></li>
> + <%if @can_modify -%>
> + <%= image_tag "icon_addhost.png", :style => "vertical-align:middle;" %>
> + <a href="<%= url_for :controller => 'vm', :action => 'new', :vm_resource_pool_id => @pool %>" rel="facebox[.bolder]">Add first virtual machine to resource pool</a></li>
> + <% end -%>
> </div>
> </div>
> </div>
> diff --git a/src/app/views/user/_grid.rhtml b/src/app/views/user/_grid.rhtml
> index cabf2af..8f7b4cf 100644
> --- a/src/app/views/user/_grid.rhtml
> +++ b/src/app/views/user/_grid.rhtml
> @@ -1,17 +1,20 @@
> <% users_per_page = 10 %>
> <div id="<%= table_id %>_div">
> -<form id="<%= table_id %>_form">
> +<%= "<form id=\"#{table_id}_form\">" if checkboxes %>
> <table id="<%= table_id %>" style="display:none"></table>
> -</form>
> +<%= '</form>' if checkboxes %>
> </div>
> <script type="text/javascript">
> $("#<%= table_id %>").flexigrid
> (
> {
> - url: '<%= url_for :controller => parent_controller, :action => "users_json", :id => pool.id %>',
> + url: '<%= url_for :controller => parent_controller,
> + :action => "users_json",
> + :id => pool.id,
> + :checkboxes => checkboxes %>',
> dataType: 'json',
> colModel : [
> - {display: '', name : 'id', width : 20, sortable : false, align: 'left', process: <%= table_id %>checkbox},
> + <%= "{display: '', width : 20, align: 'left', process: #{table_id}checkbox}," if checkboxes %>
> {display: 'Name', name : 'uid', width : 180, sortable : true, align: 'left'},
> {display: 'Role', name : 'user_role', width : 80, sortable : true, align: 'left'},
> {display: '', width : 80, sortable : true, align: 'left'}
> diff --git a/src/app/views/user/_show.rhtml b/src/app/views/user/_show.rhtml
> index 5b3ffb7..8ea423e 100644
> --- a/src/app/views/user/_show.rhtml
> +++ b/src/app/views/user/_show.rhtml
> @@ -1,8 +1,10 @@
> <div id="toolbar_nav">
> <ul>
> - <li><a href="<%= url_for :controller => 'permission', :action => 'new', :pool_id => pool.id %>" rel="facebox[.bolder]"><%= image_tag "icon_addhost.png", :style => "vertical-align:middle;" %> Add User</a></li>
> - <li><%= render :partial => 'user/change_role_menu' %></li>
> - <li><a href="#" onClick="delete_users()"><%= image_tag "icon_remove.png", :style => "vertical-align:middle;" %> Remove</a></li>
> + <%if @can_modify -%>
> + <li><a href="<%= url_for :controller => 'permission', :action => 'new', :pool_id => pool.id %>" rel="facebox[.bolder]"><%= image_tag "icon_addhost.png", :style => "vertical-align:middle;" %> Add User</a></li>
> + <li><%= render :partial => 'user/change_role_menu' %></li>
> + <li><a href="#" onClick="delete_users()"><%= image_tag "icon_remove.png", :style => "vertical-align:middle;" %> Remove</a></li>
> + <% end -%>
> </ul>
> </div>
> <script type="text/javascript">
> @@ -45,6 +47,7 @@
> <div class="data_section">
> <%= render :partial => "/user/grid", :locals => { :table_id => "users_grid",
> :parent_controller => parent_controller,
> + :checkboxes => @can_modify,
> :pool => pool } %>
> <table id="users_grid" style="display:none"></table>
> </div>
> diff --git a/src/public/stylesheets/ovirt-tree/tree.css b/src/public/stylesheets/ovirt-tree/tree.css
> index ebfa3ba..3ee0fcc 100644
> --- a/src/public/stylesheets/ovirt-tree/tree.css
> +++ b/src/public/stylesheets/ovirt-tree/tree.css
> @@ -4,9 +4,8 @@
>
> .nav-tree {
> width: 222px;
> - position: absolute;
> overflow: auto;
> - top: 51px;
> + position: relative;
> }
>
> .ovirt-tree, .ovirt-tree ul {
> @@ -93,7 +92,7 @@
> background-repeat: no-repeat;
> background-position: left;
> padding: 4px 0 4px 28px;
> - position: absolute;
> + position: relative;
> }
>
> .nav-networks {
> @@ -101,6 +100,5 @@
> background-repeat: no-repeat;
> background-position: left;
> padding: 4px 0 4px 28px;
> - position:absolute;
> - top:28px;
> + position:relative;
> }
+1 for getting rid of some absolute positioning!
More information about the ovirt-devel
mailing list