[PATCH conductor] bug 798516: prevent last admin deletion

Tzu-Mainn Chen tzumainn at redhat.com
Wed Sep 5 23:46:27 UTC 2012


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" <sseago at redhat.com>
> To: aeolus-devel at lists.fedorahosted.org
> Cc: "Scott Seago" <sseago at redhat.com>
> 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
> 
> 



More information about the aeolus-devel mailing list