Scott Seago
2009-Jan-22 23:01 UTC
[Ovirt-devel] [PATCH] more permissions validation for non-privileged users.
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
-
def addstorage
- add_internal
render :layout => 'popup'
end
def add
- add_internal
render :layout => false
end
def new
- add_internal
render :layout => false
end
@@ -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
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 %>
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.6.0.6
Jason Guiditta
2009-Jan-26 21:56 UTC
[Ovirt-devel] [PATCH] more permissions validation for non-privileged users.
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 > endIs 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 > endpre_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!