[PATCH conductor] BZ788048: Fix permission checking on pools/_images

Imre Farkas ifarkas at redhat.com
Thu Mar 1 10:27:09 UTC 2012


On 02/29/2012 05:59 PM, Scott Seago wrote:
> On 02/29/2012 04:23 AM, ifarkas at redhat.com wrote:
>> From: Imre Farkas<ifarkas at redhat.com>
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=788048
>> ---
>> src/app/views/pools/_images.html.haml | 33
>> +++++++++++++++++----------------
>> 1 files changed, 17 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/app/views/pools/_images.html.haml
>> b/src/app/views/pools/_images.html.haml
>> index c580d96..cf5513d 100644
>> --- a/src/app/views/pools/_images.html.haml
>> +++ b/src/app/views/pools/_images.html.haml
>> @@ -5,21 +5,22 @@
>> =image_tag 'flash_error_icon.png', :alt => 'Errors'
>> =flash[:error]
>> -else
>> - - @pool.catalogs.each do |catalog|
>> - %div.catalog_deployments_and_images
>> - %h1
>> - = catalog.name
>> - = t('.catalog')
>> - - catalog.deployables.each do |deployable|
>> - %h2
>> - = deployable.name
>> - = t('.deployable')
>> + - if check_privilege(Privilege::VIEW, PoolFamily, @pool.pool_family)
> You should verify permissions on deployables instead.
>
> Better yet, filter on catalog permissions. Remove the chec-privilege
> line above and...
>> + - @pool.catalogs.each do |catalog|
> Change this to @pool.catalogs.list_for_user(current_user).each...
>
> Permissions are per-catalog (although in practice pool user will have
> all these permissions anyway
>
> Scott
>
>
>> + %div.catalog_deployments_and_images
>> + %h1
>> + = catalog.name
>> + = t('.catalog')
>> + - catalog.deployables.each do |deployable|
>> + %h2
>> + = deployable.name
>> + = t('.deployable')
>>
>> - - deployable.fetch_images.compact.each do |image|
>> - %h3
>> - = t('.image')
>> - = "#{image.name} (#{image.uuid})"
>> + - deployable.fetch_images.compact.each do |image|
>> + %h3
>> + = t('.image')
>> + = "#{image.name} (#{image.uuid})"
>>
>> - %ul
>> - - image.provider_images.each do |provider_image|
>> - %li= t('.pushed', :uuid => provider_image.uuid, :provider =>
>> provider_image.provider_name)
>> + %ul
>> + - image.provider_images.each do |provider_image|
>> + %li= t('.pushed', :uuid => provider_image.uuid, :provider =>
>> provider_image.provider_name)
>

Thanks for the review! I will send a new patch.
Imre



More information about the aeolus-devel mailing list