ACK'd and pushed. The BZ comments mentioned a potential flash message once a user takes away his own admin rights; I'm guessing that was deemed unnecessary. . . ?
Mainn ----- Original Message ----- > From: "Scott Seago" <[email protected]> > To: [email protected] > Cc: "Scott Seago" <[email protected]> > Sent: Wednesday, September 5, 2012 10:41:05 AM > Subject: [PATCH conductor] bug 798516: prevent last admin deletion > > https://bugzilla.redhat.com/show_bug.cgi?id=798516 > > Don't allow deletion of admin permissions if there is only one admin > left. > Upon permission removal, redirect user to /account if the user's > own permissions to that page have been revoked. > --- > src/app/controllers/application_controller.rb | 5 +++++ > src/app/controllers/permissions_controller.rb | 6 ++++-- > src/app/models/base_permission_object.rb | 13 > +++++++++++++ > src/app/views/permissions/_permissions.html.haml | 6 ++++-- > src/spec/controllers/permissions_controller_spec.rb | 2 +- > 5 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/src/app/controllers/application_controller.rb > b/src/app/controllers/application_controller.rb > index 2d7f325..64f3498 100644 > --- a/src/app/controllers/application_controller.rb > +++ b/src/app/controllers/application_controller.rb > @@ -44,6 +44,11 @@ class ApplicationController < > ActionController::Base > # permissions checking > > def handle_perm_error(error) > + if params[:return_from_permission_change] > + flash.now > + redirect_to account_url > + return > + end > handle_error(:error => error, :status => :forbidden, > :title => > t('application_controller.access_denied')) > end > diff --git a/src/app/controllers/permissions_controller.rb > b/src/app/controllers/permissions_controller.rb > index 33c2a7e..0bc90cb 100644 > --- a/src/app/controllers/permissions_controller.rb > +++ b/src/app/controllers/permissions_controller.rb > @@ -220,7 +220,7 @@ class PermissionsController < > ApplicationController > raise RuntimeError, "invalid permission object" if > @permission_object.nil? > unless @return_path > if @permission_object == > BasePermissionObject.general_permission_scope > - @return_path = permissions_path > + @return_path = > permissions_path(:return_from_permission_change => true) > set_admin_users_tabs 'permissions' > else > @return_path = send("#{@path_prefix}polymorphic_path", > @@ -230,7 +230,9 @@ class PermissionsController < > ApplicationController > @polymorphic_path_extras) : > @permission_object, > @use_tabs == "yes" ? {:details_tab => > :permissions, > - :only_tab => true} : {}) > + :only_tab => true, > + :return_from_permission_change => > true} : > + {:return_from_permission_change => > true}) > end > end > require_privilege(required_role, @permission_object) > diff --git a/src/app/models/base_permission_object.rb > b/src/app/models/base_permission_object.rb > index c5d525a..faf5e53 100644 > --- a/src/app/models/base_permission_object.rb > +++ b/src/app/models/base_permission_object.rb > @@ -56,4 +56,17 @@ class BasePermissionObject < ActiveRecord::Base > [HardwareProfile, Catalog, Deployable, PoolFamily, Pool, > Deployment, Instance, Provider, ProviderAccount] > end > + > + def self.global_admin_permission_count > + self.general_permission_scope.permissions.includes(:role => > :privileges). > + where("privileges.target_type" => "BasePermissionObject", > + "privileges.action" => Privilege::PERM_SET).size > + end > + > + def self.is_global_admin_perm(permission) > + permission.role.privileges.where("privileges.target_type" => > + "BasePermissionObject", > + "privileges.action" => > + Privilege::PERM_SET).size > 0 > + end > end > diff --git a/src/app/views/permissions/_permissions.html.haml > b/src/app/views/permissions/_permissions.html.haml > index 4f2818d..6d8ab32 100644 > --- a/src/app/views/permissions/_permissions.html.haml > +++ b/src/app/views/permissions/_permissions.html.haml > @@ -1,3 +1,5 @@ > +- has_admin_perms = check_privilege(Privilege::PERM_SET) > +- prevent_admin_deletion = has_admin_perms && !(@show_inherited || > @show_global) && (@permission_object == > BasePermissionObject.general_permission_scope) && > (BasePermissionObject.global_admin_permission_count == 1) > - content_for :permissions_form_header do > - if not(@show_inherited or @show_global) && > check_privilege(Privilege::PERM_SET) > %li= link_to t('permissions.form.grant_access'), > new_permission_path(:permission_object_type => > @permission_object.class.name, :permission_object_id => > @permission_object.id, :path_prefix => @path_prefix, :use_tabs > => @use_tabs ? @use_tabs : (@tabs ? 'yes' : 'no')), { :class => > 'button primary', :id => 'new_permission_button'} > @@ -51,11 +53,11 @@ > %tr{:class => cycle('nostripe','stripe')} > - if not(@show_inherited or @show_global) > %td{:class => 'checkbox'} > - - if check_privilege(Privilege::PERM_SET) > + - if has_admin_perms && !(prevent_admin_deletion && > BasePermissionObject.is_global_admin_perm(permission)) > - selected = params[:select] == 'all' > = check_box_tag "permission_selected[]", permission.id, > selected, :id => "permission_checkbox_#{permission.id}" > %td= link_to permission.entity.name, > ((permission.entity.entity_target_type == "User") ? > user_path(permission.entity.user) : > user_group_path(permission.entity.user_group)) > - - if not(@show_inherited or @show_global) && > check_privilege(Privilege::PERM_SET) > + - if not(@show_inherited or @show_global) && has_admin_perms && > !(prevent_admin_deletion && > BasePermissionObject.is_global_admin_perm(permission)) > %td= select_tag "permission_role_selected[]", > options_for_select(@roles.map {|r| [ t(r.name, :scope=> > :role_defs, :default => r.name), "#{permission.id},#{r.id}" ] > }, :selected => "#{permission.id},#{permission.role.id}", > :disabled => @permission_object.permissions.where(["entity_id > = ? and role_id != ?", permission.entity_id, > permission.role_id]).collect {|p| > "#{permission.id},#{p.role.id}"}), :id => > "permission_role_selected_#{permission.id}" > :javascript > $("#permission_role_selected_#{permission.id}").change(function > () { $("#perm_edit_button").click(); } ); > diff --git a/src/spec/controllers/permissions_controller_spec.rb > b/src/spec/controllers/permissions_controller_spec.rb > index a27d669..4538dc9 100644 > --- a/src/spec/controllers/permissions_controller_spec.rb > +++ b/src/spec/controllers/permissions_controller_spec.rb > @@ -36,7 +36,7 @@ describe PermissionsController do > post :multi_update, :permission_object_id => @deployable.id, > :permission_object_type => @deployable.class.to_s, > :permission_role_selected => > ["#{@permission.id},#{@new_role.id}"], > :polymorphic_path_extras => { 'catalog_id' => @catalog.id} > > - response.should redirect_to catalog_deployable_path(@catalog, > @deployable) > + response.should redirect_to catalog_deployable_path(@catalog, > @deployable, :return_from_permission_change => true) > end > > it "should work for global role grants" do > -- > 1.7.11.4 > >
