Permission records are now stored for all pools a user has access to, even implied/inherited permissions. This means that if an admin is granted access to the 'default' pool, there will be one permission record for this pool, and an additional "inherited" record for each pool below 'default' in the hierarchy -- for the rest of these records, inherited_from_id will point to the master permission record. Any time a user is granted permission on a pool, the inherited records are automatically created. When a new pool is created, inherited records are created here too. You need to run 'rake db:migrate' to get the updated permissions table -- the migration script will take care of creating inherited records for existing permissions. In the UI, both direct and inherited grants are shown, but only direct grants have the checkbox to allow modification/deletion. Signed-off-by: Scott Seago <sseago at redhat.com> --- wui/src/app/controllers/hardware_controller.rb | 2 +- wui/src/app/controllers/permission_controller.rb | 14 +++--- wui/src/app/controllers/resources_controller.rb | 2 +- wui/src/app/models/permission.rb | 42 ++++++++++++++++- wui/src/app/models/pool.rb | 35 ++++++++++----- wui/src/app/views/user/_grid.rhtml | 51 +++++++++++--------- wui/src/db/migrate/012_denormalize_permissions.rb | 32 +++++++++++++ 7 files changed, 133 insertions(+), 45 deletions(-) create mode 100644 wui/src/db/migrate/012_denormalize_permissions.rb diff --git a/wui/src/app/controllers/hardware_controller.rb b/wui/src/app/controllers/hardware_controller.rb index e105f7c..5f473db 100644 --- a/wui/src/app/controllers/hardware_controller.rb +++ b/wui/src/app/controllers/hardware_controller.rb @@ -151,7 +151,7 @@ class HardwareController < ApplicationController def users_json json_list(@pool.permissions, - [:id, :uid, :user_role]) + [:grid_id, :uid, :user_role, :source]) end def storage_pools_json diff --git a/wui/src/app/controllers/permission_controller.rb b/wui/src/app/controllers/permission_controller.rb index 14358de..4367275 100644 --- a/wui/src/app/controllers/permission_controller.rb +++ b/wui/src/app/controllers/permission_controller.rb @@ -59,10 +59,11 @@ class PermissionController < ApplicationController flash[:notice] = 'You do not have permission to create this permission record' redirect_to_parent else - if @permission.save + begin + @permission.save_with_new_children render :json => "created User Permissions for #{@permission.uid}".to_json - else - # FIXME: need to handle proper error messages w/ ajax + rescue + # FIXME: need to handle proper error messages w/ ajax render :action => 'new' end end @@ -79,13 +80,12 @@ class PermissionController < ApplicationController Permission.transaction do permissions = Permission.find(:all, :conditions => "id in (#{permission_ids.join(', ')})") permissions.each do |permission| - permission.user_role = role - permission.save! + permission.update_role(role) if permission.is_primary? end end render :json => { :object => "permission", :success => true, :alert => "User roles were successfully updated." } - rescue + rescue render :json => { :object => "permission", :success => false, :alert => "Error updating user roles: #{$!}" } end @@ -101,7 +101,7 @@ class PermissionController < ApplicationController Permission.transaction do permissions = Permission.find(:all, :conditions => "id in (#{permission_ids.join(', ')})") permissions.each do |permission| - permission.destroy + permission.destroy if permission.is_primary? end end render :json => { :object => "permission", :success => true, diff --git a/wui/src/app/controllers/resources_controller.rb b/wui/src/app/controllers/resources_controller.rb index dbf9ad7..20defca 100644 --- a/wui/src/app/controllers/resources_controller.rb +++ b/wui/src/app/controllers/resources_controller.rb @@ -97,7 +97,7 @@ class ResourcesController < ApplicationController def users_json json_list(@vm_resource_pool.permissions, - [:id, :uid, :user_role]) + [:grid_id, :uid, :user_role, :source]) end def new diff --git a/wui/src/app/models/permission.rb b/wui/src/app/models/permission.rb index 74c6c26..ece3da5 100644 --- a/wui/src/app/models/permission.rb +++ b/wui/src/app/models/permission.rb @@ -19,8 +19,13 @@ class Permission < ActiveRecord::Base belongs_to :pool + belongs_to :parent_permission, :class_name => "Permission", + :foreign_key => "inherited_from_id" + has_many :child_permissions, :dependent => :destroy, + :class_name => "Permission", :foreign_key => "inherited_from_id" - validates_uniqueness_of :uid, :scope => "pool_id" + + validates_uniqueness_of :uid, :scope => [:pool_id, :inherited_from_id] ROLE_SUPER_ADMIN = "Super Admin" ROLE_ADMIN = "Administrator" @@ -68,5 +73,38 @@ class Permission < ActiveRecord::Base PRIVILEGES[privilege] end - + def is_primary? + inherited_from_id.nil? + end + def is_inherited? + !is_primary? + end + def source + is_primary? ? "Direct" : "Inherited" + end + def grid_id + id.to_s + "_" + (is_primary? ? "1" : "0") + end + def update_role(new_role) + self.transaction do + self.user_role = new_role + self.save! + child_permissions.each do |permission| + permission.user_role = new_role + permission.save! + end + end + end + def save_with_new_children + self.transaction do + self.save! + pool.all_children.each do |subpool| + new_permission = Permission.new({:pool_id => subpool.id, + :uid => uid, + :user_role => user_role, + :inherited_from_id => id}) + new_permission.save! + end + end + end end diff --git a/wui/src/app/models/pool.rb b/wui/src/app/models/pool.rb index 4aa240b..74fe698 100644 --- a/wui/src/app/models/pool.rb +++ b/wui/src/app/models/pool.rb @@ -71,18 +71,33 @@ class Pool < ActiveRecord::Base transaction do save! move_to_child_of(parent) + parent.permissions.each do |permission| + new_permission = Permission.new({:pool_id => id, + :uid => permission.uid, + :user_role => permission.user_role, + :inherited_from_id => + permission.inherited_from_id.nil? ? + permission.id : + permission.inherited_from_id}) + new_permission.save! + end yield other_actions if other_actions end end acts_as_xapian :texts => [ :name ] - # this method lists pools with direct permission grants, but does not - # include implied permissions (i.e. subtrees) - def self.list_for_user(user, privilege) - pools = find(:all, :include => "permissions", - :conditions => "permissions.uid='#{user}' and - permissions.user_role in + # this method lists pools with direct permission grants, but by default does + # not include implied permissions (i.e. subtrees) + def self.list_for_user(user, privilege, include_indirect = false) + if include_indirect + inherited_clause = "" + else + inherited_clause = "and permissions.inherited_from_id is null" + end + pools = find(:all, :include => "permissions", + :conditions => "permissions.uid='#{user}' #{inherited_clause} and + permissions.user_role in ('#{Permission.roles_for_privilege(privilege).join("', '")}')") end @@ -126,12 +141,10 @@ class Pool < ActiveRecord::Base end def has_privilege(user, privilege) - traverse_parents do |pool| - pool.permissions.find(:first, - :conditions => "permissions.uid='#{user}' and - permissions.user_role in + pool.permissions.find(:first, + :conditions => "permissions.uid='#{user}' and + permissions.user_role in ('#{Permission.roles_for_privilege(privilege).join("', '")}')") - end end def total_resources diff --git a/wui/src/app/views/user/_grid.rhtml b/wui/src/app/views/user/_grid.rhtml index d9ad795..cabf2af 100644 --- a/wui/src/app/views/user/_grid.rhtml +++ b/wui/src/app/views/user/_grid.rhtml @@ -5,27 +5,32 @@ </form> </div> <script type="text/javascript"> - $("#<%= table_id %>").flexigrid - ( - { - url: '<%= url_for :controller => parent_controller, :action => "users_json", :id => pool.id %>', - dataType: 'json', - colModel : [ - {display: '', name : 'id', width : 20, sortable : false, align: 'left', process: <%= table_id %>checkbox}, - {display: 'Name', name : 'uid', width : 180, sortable : true, align: 'left'}, - {display: 'Role', name : 'user_role', width : 80, sortable : true, align: 'left'} - ], - sortname: "user", - sortorder: "asc", - usepager: <%= pool.permissions.size > users_per_page ? 'true' : 'false' %>, - useRp: true, - rp: <%= users_per_page %>, - showTableToggleBtn: true - } - ); - function <%= table_id %>checkbox(celDiv) - { - $(celDiv).html('<input type="checkbox" name="grid_checkbox'+$(celDiv).html()+'" class="grid_checkbox" value="'+$(celDiv).html()+'"/>'); - } - + $("#<%= table_id %>").flexigrid + ( + { + url: '<%= url_for :controller => parent_controller, :action => "users_json", :id => pool.id %>', + dataType: 'json', + colModel : [ + {display: '', name : 'id', width : 20, sortable : false, align: 'left', process: <%= table_id %>checkbox}, + {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'} + ], + sortname: "user", + sortorder: "asc", + usepager: <%= pool.permissions.size > users_per_page ? 'true' : 'false' %>, + useRp: true, + rp: <%= users_per_page %>, + showTableToggleBtn: true + } + ); + function <%= table_id %>checkbox(celDiv) + { + var grid_id = $(celDiv).html().split("_"); + if (grid_id[1] == 1) { + $(celDiv).html('<input type="checkbox" name="grid_checkbox'+grid_id[0]+'" class="grid_checkbox" value="'+grid_id[0]+'"/>'); + } else { + $(celDiv).html('') + } + } </script> diff --git a/wui/src/db/migrate/012_denormalize_permissions.rb b/wui/src/db/migrate/012_denormalize_permissions.rb new file mode 100644 index 0000000..f68f555 --- /dev/null +++ b/wui/src/db/migrate/012_denormalize_permissions.rb @@ -0,0 +1,32 @@ +class DenormalizePermissions < ActiveRecord::Migration + def self.up + add_column :permissions, :inherited_from_id, :integer + execute "alter table permissions add constraint fk_perm_parent + foreign key (inherited_from_id) references permissions(id)" + + Permission.transaction do + Permission.find(:all, + :conditions => "inherited_from_id is null" + ).each do |permission| + permission.pool.all_children.each do |subpool| + new_permission = Permission.new({:pool_id => subpool.id, + :uid => permission.uid, + :user_role => permission.user_role, + :inherited_from_id => permission.id}) + new_permission.save! + end + end + end + end + + def self.down + Permission.transaction do + Permission.find(:all, + :conditions => "inherited_from_id is not null" + ).each do |permission| + permission.destroy + end + end + remove_column :permissions, :inherited_from_id + end +end -- 1.5.5.1
Permission records are now stored for all pools a user has access to, even implied/inherited permissions. This means that if an admin is granted access to the 'default' pool, there will be one permission record for this pool, and an additional "inherited" record for each pool below 'default' in the hierarchy -- for the rest of these records, inherited_from_id will point to the master permission record. Any time a user is granted permission on a pool, the inherited records are automatically created. When a new pool is created, inherited records are created here too. You need to run 'rake db:migrate' to get the updated permissions table -- the migration script will take care of creating inherited records for existing permissions. In the UI, both direct and inherited grants are shown, but only direct grants have the checkbox to allow modification/deletion. Signed-off-by: Scott Seago <sseago at redhat.com> --- wui/src/app/controllers/hardware_controller.rb | 2 +- wui/src/app/controllers/permission_controller.rb | 14 +++--- wui/src/app/controllers/resources_controller.rb | 2 +- wui/src/app/models/permission.rb | 42 ++++++++++++++++- wui/src/app/models/pool.rb | 35 ++++++++++----- wui/src/app/views/user/_grid.rhtml | 51 +++++++++++--------- wui/src/db/migrate/012_denormalize_permissions.rb | 32 +++++++++++++ 7 files changed, 133 insertions(+), 45 deletions(-) create mode 100644 wui/src/db/migrate/012_denormalize_permissions.rb diff --git a/wui/src/app/controllers/hardware_controller.rb b/wui/src/app/controllers/hardware_controller.rb index e105f7c..5f473db 100644 --- a/wui/src/app/controllers/hardware_controller.rb +++ b/wui/src/app/controllers/hardware_controller.rb @@ -151,7 +151,7 @@ class HardwareController < ApplicationController def users_json json_list(@pool.permissions, - [:id, :uid, :user_role]) + [:grid_id, :uid, :user_role, :source]) end def storage_pools_json diff --git a/wui/src/app/controllers/permission_controller.rb b/wui/src/app/controllers/permission_controller.rb index 14358de..4367275 100644 --- a/wui/src/app/controllers/permission_controller.rb +++ b/wui/src/app/controllers/permission_controller.rb @@ -59,10 +59,11 @@ class PermissionController < ApplicationController flash[:notice] = 'You do not have permission to create this permission record' redirect_to_parent else - if @permission.save + begin + @permission.save_with_new_children render :json => "created User Permissions for #{@permission.uid}".to_json - else - # FIXME: need to handle proper error messages w/ ajax + rescue + # FIXME: need to handle proper error messages w/ ajax render :action => 'new' end end @@ -79,13 +80,12 @@ class PermissionController < ApplicationController Permission.transaction do permissions = Permission.find(:all, :conditions => "id in (#{permission_ids.join(', ')})") permissions.each do |permission| - permission.user_role = role - permission.save! + permission.update_role(role) if permission.is_primary? end end render :json => { :object => "permission", :success => true, :alert => "User roles were successfully updated." } - rescue + rescue render :json => { :object => "permission", :success => false, :alert => "Error updating user roles: #{$!}" } end @@ -101,7 +101,7 @@ class PermissionController < ApplicationController Permission.transaction do permissions = Permission.find(:all, :conditions => "id in (#{permission_ids.join(', ')})") permissions.each do |permission| - permission.destroy + permission.destroy if permission.is_primary? end end render :json => { :object => "permission", :success => true, diff --git a/wui/src/app/controllers/resources_controller.rb b/wui/src/app/controllers/resources_controller.rb index dbf9ad7..20defca 100644 --- a/wui/src/app/controllers/resources_controller.rb +++ b/wui/src/app/controllers/resources_controller.rb @@ -97,7 +97,7 @@ class ResourcesController < ApplicationController def users_json json_list(@vm_resource_pool.permissions, - [:id, :uid, :user_role]) + [:grid_id, :uid, :user_role, :source]) end def new diff --git a/wui/src/app/models/permission.rb b/wui/src/app/models/permission.rb index 74c6c26..ece3da5 100644 --- a/wui/src/app/models/permission.rb +++ b/wui/src/app/models/permission.rb @@ -19,8 +19,13 @@ class Permission < ActiveRecord::Base belongs_to :pool + belongs_to :parent_permission, :class_name => "Permission", + :foreign_key => "inherited_from_id" + has_many :child_permissions, :dependent => :destroy, + :class_name => "Permission", :foreign_key => "inherited_from_id" - validates_uniqueness_of :uid, :scope => "pool_id" + + validates_uniqueness_of :uid, :scope => [:pool_id, :inherited_from_id] ROLE_SUPER_ADMIN = "Super Admin" ROLE_ADMIN = "Administrator" @@ -68,5 +73,38 @@ class Permission < ActiveRecord::Base PRIVILEGES[privilege] end - + def is_primary? + inherited_from_id.nil? + end + def is_inherited? + !is_primary? + end + def source + is_primary? ? "Direct" : "Inherited" + end + def grid_id + id.to_s + "_" + (is_primary? ? "1" : "0") + end + def update_role(new_role) + self.transaction do + self.user_role = new_role + self.save! + child_permissions.each do |permission| + permission.user_role = new_role + permission.save! + end + end + end + def save_with_new_children + self.transaction do + self.save! + pool.all_children.each do |subpool| + new_permission = Permission.new({:pool_id => subpool.id, + :uid => uid, + :user_role => user_role, + :inherited_from_id => id}) + new_permission.save! + end + end + end end diff --git a/wui/src/app/models/pool.rb b/wui/src/app/models/pool.rb index 4aa240b..74fe698 100644 --- a/wui/src/app/models/pool.rb +++ b/wui/src/app/models/pool.rb @@ -71,18 +71,33 @@ class Pool < ActiveRecord::Base transaction do save! move_to_child_of(parent) + parent.permissions.each do |permission| + new_permission = Permission.new({:pool_id => id, + :uid => permission.uid, + :user_role => permission.user_role, + :inherited_from_id => + permission.inherited_from_id.nil? ? + permission.id : + permission.inherited_from_id}) + new_permission.save! + end yield other_actions if other_actions end end acts_as_xapian :texts => [ :name ] - # this method lists pools with direct permission grants, but does not - # include implied permissions (i.e. subtrees) - def self.list_for_user(user, privilege) - pools = find(:all, :include => "permissions", - :conditions => "permissions.uid='#{user}' and - permissions.user_role in + # this method lists pools with direct permission grants, but by default does + # not include implied permissions (i.e. subtrees) + def self.list_for_user(user, privilege, include_indirect = false) + if include_indirect + inherited_clause = "" + else + inherited_clause = "and permissions.inherited_from_id is null" + end + pools = find(:all, :include => "permissions", + :conditions => "permissions.uid='#{user}' #{inherited_clause} and + permissions.user_role in ('#{Permission.roles_for_privilege(privilege).join("', '")}')") end @@ -126,12 +141,10 @@ class Pool < ActiveRecord::Base end def has_privilege(user, privilege) - traverse_parents do |pool| - pool.permissions.find(:first, - :conditions => "permissions.uid='#{user}' and - permissions.user_role in + pool.permissions.find(:first, + :conditions => "permissions.uid='#{user}' and + permissions.user_role in ('#{Permission.roles_for_privilege(privilege).join("', '")}')") - end end def total_resources diff --git a/wui/src/app/views/user/_grid.rhtml b/wui/src/app/views/user/_grid.rhtml index d9ad795..cabf2af 100644 --- a/wui/src/app/views/user/_grid.rhtml +++ b/wui/src/app/views/user/_grid.rhtml @@ -5,27 +5,32 @@ </form> </div> <script type="text/javascript"> - $("#<%= table_id %>").flexigrid - ( - { - url: '<%= url_for :controller => parent_controller, :action => "users_json", :id => pool.id %>', - dataType: 'json', - colModel : [ - {display: '', name : 'id', width : 20, sortable : false, align: 'left', process: <%= table_id %>checkbox}, - {display: 'Name', name : 'uid', width : 180, sortable : true, align: 'left'}, - {display: 'Role', name : 'user_role', width : 80, sortable : true, align: 'left'} - ], - sortname: "user", - sortorder: "asc", - usepager: <%= pool.permissions.size > users_per_page ? 'true' : 'false' %>, - useRp: true, - rp: <%= users_per_page %>, - showTableToggleBtn: true - } - ); - function <%= table_id %>checkbox(celDiv) - { - $(celDiv).html('<input type="checkbox" name="grid_checkbox'+$(celDiv).html()+'" class="grid_checkbox" value="'+$(celDiv).html()+'"/>'); - } - + $("#<%= table_id %>").flexigrid + ( + { + url: '<%= url_for :controller => parent_controller, :action => "users_json", :id => pool.id %>', + dataType: 'json', + colModel : [ + {display: '', name : 'id', width : 20, sortable : false, align: 'left', process: <%= table_id %>checkbox}, + {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'} + ], + sortname: "user", + sortorder: "asc", + usepager: <%= pool.permissions.size > users_per_page ? 'true' : 'false' %>, + useRp: true, + rp: <%= users_per_page %>, + showTableToggleBtn: true + } + ); + function <%= table_id %>checkbox(celDiv) + { + var grid_id = $(celDiv).html().split("_"); + if (grid_id[1] == 1) { + $(celDiv).html('<input type="checkbox" name="grid_checkbox'+grid_id[0]+'" class="grid_checkbox" value="'+grid_id[0]+'"/>'); + } else { + $(celDiv).html('') + } + } </script> diff --git a/wui/src/db/migrate/012_denormalize_permissions.rb b/wui/src/db/migrate/012_denormalize_permissions.rb new file mode 100644 index 0000000..f68f555 --- /dev/null +++ b/wui/src/db/migrate/012_denormalize_permissions.rb @@ -0,0 +1,32 @@ +class DenormalizePermissions < ActiveRecord::Migration + def self.up + add_column :permissions, :inherited_from_id, :integer + execute "alter table permissions add constraint fk_perm_parent + foreign key (inherited_from_id) references permissions(id)" + + Permission.transaction do + Permission.find(:all, + :conditions => "inherited_from_id is null" + ).each do |permission| + permission.pool.all_children.each do |subpool| + new_permission = Permission.new({:pool_id => subpool.id, + :uid => permission.uid, + :user_role => permission.user_role, + :inherited_from_id => permission.id}) + new_permission.save! + end + end + end + end + + def self.down + Permission.transaction do + Permission.find(:all, + :conditions => "inherited_from_id is not null" + ).each do |permission| + permission.destroy + end + end + remove_column :permissions, :inherited_from_id + end +end -- 1.5.5.1