[PATCH conductor] bug 808031: allow Global Provider user to view Provider Accounts

Jan Provaznik jprovazn at redhat.com
Mon May 28 10:18:44 UTC 2012


On 05/25/2012 07:33 PM, Scott Seago wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=808031
>
> For provider accounts, the 'edit' action doubles as the 'show' page.
> This meant we were requiring modify privileges on the provider to even
> see the provider pages. This patch relaxes the checking to reqiure
> only view permissions, and in the views we hide the edit/modify actions
> from users that don't have the higher privileges, and the form elements
> are made read-only.
> ---
>   .../controllers/provider_accounts_controller.rb    |    1 +
>   src/app/controllers/providers_controller.rb        |    4 +++-
>   src/app/views/provider_accounts/_list.html.haml    |    3 ++-
>   src/app/views/providers/_edit.html.haml            |   12 +++++++-----
>   src/app/views/providers/_form.html.haml            |    8 ++++----
>   src/app/views/providers/new.html.haml              |    2 +-
>   6 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/src/app/controllers/provider_accounts_controller.rb b/src/app/controllers/provider_accounts_controller.rb
> index bf48af3..4aeee9c 100644
> --- a/src/app/controllers/provider_accounts_controller.rb
> +++ b/src/app/controllers/provider_accounts_controller.rb
> @@ -69,6 +69,7 @@ class ProviderAccountsController<  ApplicationController
>       else
>         @provider = Provider.find(params[:provider_id])
>       end
> +    require_privilege(Privilege::CREATE, ProviderAccount, @provider)
r
^ if there are no providers, @provider is unset - this check might be in 
the 'else' block 1 row upper

>     end
>
>     def create
> diff --git a/src/app/controllers/providers_controller.rb b/src/app/controllers/providers_controller.rb
> index 64a334f..e96ec34 100644
> --- a/src/app/controllers/providers_controller.rb
> +++ b/src/app/controllers/providers_controller.rb
> @@ -55,7 +55,9 @@ class ProvidersController<  ApplicationController
>       @provider = Provider.find_by_id(params[:id])
>       @title = t 'cloud_providers'
>       session[:current_provider_id] = @provider.id
> -    require_privilege(Privilege::MODIFY, @provider)
> +    # requiring VIEW rather than MODIFY since edit doubles as the 'show' page
> +    # here -- actions must be hidden explicitly in template
> +    require_privilege(Privilege::VIEW, @provider)
>
>       @alerts = provider_alerts(@provider)
>
> diff --git a/src/app/views/provider_accounts/_list.html.haml b/src/app/views/provider_accounts/_list.html.haml
> index 60e217d..31f73c7 100644
> --- a/src/app/views/provider_accounts/_list.html.haml
> +++ b/src/app/views/provider_accounts/_list.html.haml
> @@ -1,7 +1,8 @@
>   .content
>     - content_for :form_header do
>       %li= restful_submit_tag t('delete'), "destroy", multi_destroy_provider_provider_accounts_path(@provider), 'DELETE', :id =>  'delete_button', :class =>  'button danger'
> -    %li= link_to t('provider_accounts.new_provider_account'), new_provider_provider_account_path(@provider), :id =>  'new_provider_account', :class =>  'button'
> +    - if check_privilege(Privilege::CREATE, ProviderAccount, @provider)
> +      %li= link_to t('provider_accounts.new_provider_account'), new_provider_provider_account_path(@provider), :id =>  'new_provider_account', :class =>  'button'
>
>     - content_for :filter_controls do
>       %li
> diff --git a/src/app/views/providers/_edit.html.haml b/src/app/views/providers/_edit.html.haml
> index 8ba45fd..22ab0db 100644
> --- a/src/app/views/providers/_edit.html.haml
> +++ b/src/app/views/providers/_edit.html.haml
> @@ -1,8 +1,10 @@
>   .content
>     = form_for @provider, :url =>  provider_path(@provider), :html =>  { :method =>  :put, :class =>  'generic horizontal' } do |f|
> -    = render :partial =>  'form', :locals =>  { :form =>  f }
> +    - can_edit = check_privilege(Privilege::MODIFY, @provider)
> +    = render :partial =>  'form', :locals =>  { :form =>  f, :readonly =>  !can_edit }
>       %fieldset.options
> -      .button-group
> -        = f.submit "Save Changes", :class =>  "submit button pill", :id =>  'save'
> -        = link_to "Test Connection", edit_provider_path(@provider, :test_provider =>  true), :class =>  'button pill'
> -        = link_to 'Delete Provider', provider_path(@provider), :method =>  :delete, :confirm =>  t('providers.confirm_delete'), :class =>  "button pill danger", :id =>  "delete"
> +      - if can_edit
> +        .button-group
> +          = f.submit "Save Changes", :class =>  "submit button pill", :id =>  'save'
> +          = link_to "Test Connection", edit_provider_path(@provider, :test_provider =>  true), :class =>  'button pill'
> +          = link_to 'Delete Provider', provider_path(@provider), :method =>  :delete, :confirm =>  t('providers.confirm_delete'), :class =>  "button pill danger", :id =>  "delete"
> diff --git a/src/app/views/providers/_form.html.haml b/src/app/views/providers/_form.html.haml
> index 387d952..480e31b 100644
> --- a/src/app/views/providers/_form.html.haml
> +++ b/src/app/views/providers/_form.html.haml
> @@ -4,19 +4,19 @@
>     .field
>       = form.label :name, :class =>  'em'
>       .input
> -      = form.text_field :name, :value =>  @provider.name, :class =>  'em long'
> +      = form.text_field :name, :value =>  @provider.name, :class =>  'em long', :readonly =>  readonly
>     .field
>       = form.label :url
>       .input
> -      = form.text_field :url, :value =>  @provider.url, :class =>  'long'
> +      = form.text_field :url, :value =>  @provider.url, :class =>  'long', :readonly =>  readonly
>     .field
>       = form.label :provider_type_id
>       .input
> -      = form.select(:provider_type_id, @provider_type_options, :prompt =>  t('providers.form.select_type_of_provider'))
> +      = form.select(:provider_type_id, @provider_type_options, {:prompt =>  t('providers.form.select_type_of_provider')}, {:disabled =>  readonly})
>     .field
>       = form.label :deltacloud_provider, t('providers.form.x_deltacloud_provider.generic')
>       .input
> -      = form.text_field :deltacloud_provider, :title =>  t('providers.form.x_deltacloud_provider.generic'), :value =>  @provider.deltacloud_provider
> +      = form.text_field :deltacloud_provider, :title =>  t('providers.form.x_deltacloud_provider.generic'), :value =>  @provider.deltacloud_provider, :readonly =>  readonly
>     .field#x-deltacloud-provider-legend
>       %label= t('providers.form.x_deltacloud_provider_legend')
>       .input
> diff --git a/src/app/views/providers/new.html.haml b/src/app/views/providers/new.html.haml
> index bcac552..b2465b1 100644
> --- a/src/app/views/providers/new.html.haml
> +++ b/src/app/views/providers/new.html.haml
> @@ -16,6 +16,6 @@
>       %h2.settings= t("settings.settings")
>     .content
>       = form_for @provider, :url =>  providers_path, :html =>  {:class =>  'generic horizontal'} do |f|
> -      = render :partial =>  "form", :locals =>  { :form =>  f, :cancel_path =>  providers_path }
> +      = render :partial =>  "form", :locals =>  { :form =>  f, :cancel_path =>  providers_path, :readonly =>  false }
>         %fieldset.options
>           = f.submit t("providers.new.save_provider"), :class =>  "submit button pill", :id =>  'save'


1 nit inline

On the providers edit page there is page_header partial with 
enable/disable button and "+ create new provider" button. When clicking 
on any of them I get "insufficient privileges..." message - these 
buttons could be displayed only if a user has proper privs too. If you 
think this as unrelated to your patch and it should be separate BZ, then 
conditional ACK to your orig patch (with one nit inline). If you are 
going to fix this too, then pls resend it and I can retest it.


Jan



More information about the aeolus-devel mailing list