Bug 699974

Summary: Allow to change Retention Tag and Product for users in same group
Product: [Retired] Beaker Reporter: Jan Stancek <jstancek>
Component: web UIAssignee: Raymond Mancy <rmancy>
Status: CLOSED CURRENTRELEASE QA Contact:
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 0.6CC: bpeck, dcallagh, ebaak, jburke, mcsontos, rmancy, stl
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-06-16 00:51:54 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Jan Stancek 2011-04-27 06:57:07 UTC
Description of problem:
As a member of kernel-mtr group I can ack/nack recipes of other people from this group, but I can't change "Retention Tag" and "Product". These 2 fields are  gray.
Initial discussion with Bill Peck suggest, that "this wasn't taken into account".

How reproducible:
Always

Steps to Reproduce:
1. as user jstancek go to: https://beaker.engineering.redhat.com/jobs/73983
  
Actual results:
Can't change "Retention Tag" and "Product"

Expected results:
Be able to change these 2 fields.

Additional info:

Comment 1 Raymond Mancy 2011-04-27 07:14:38 UTC
(In reply to comment #0)
> Description of problem:
> As a member of kernel-mtr group I can ack/nack recipes of other people from
> this group, 

That's correct. 676834 was the BZ that added this feature.

>but I can't change "Retention Tag" and "Product". These 2 fields
> are  gray.
> Initial discussion with Bill Peck suggest, that "this wasn't taken into
> account".
> 

That's right, it wasn't.

> How reproducible:
> Always
> 
> Steps to Reproduce:
> 1. as user jstancek go to: https://beaker.engineering.redhat.com/jobs/73983
> 
> Actual results:
> Can't change "Retention Tag" and "Product"
> 
> Expected results:
> Be able to change these 2 fields.
> 
> Additional info:

Comment 2 Jeff Burke 2011-06-02 15:09:58 UTC
*** Bug 710168 has been marked as a duplicate of this bug. ***

Comment 3 Bill Peck 2011-06-02 15:51:47 UTC
Two bugs here.

In the def update method this bit of code should be getting called:

        if not job.can_admin(identity.current.user):
            raise cherrypy.HTTPError(status=403,
                    message="You don't have permission to update job id %s" % id)

But the javascript is not handling this properly.

This needs to be fixed as well.

Comment 4 Raymond Mancy 2011-06-03 00:46:28 UTC
There are a couple of issues here.


1) Bill that code block you referred to was actually put there to stop people
   trying to hit the URL directly and do something they weren't supposed to. Not
   to pass on an error msg to any JS that may have called it 
   (the JS should leave the interface grey for those who don't have access 
   so they should have no need for that msg..which brings me to point 2)

2) can_access() is used to determine if they can change the input, 
   however can_admin() was being used to check the perms. Jeff can get past
   can_access() (as a queue_admin) but not past can_admin(). So that's a bug.

The way I'm using can_access() doesn't really make sense, so I'm quite happy to completely replace it with can_admin() and change the definition of can_admin() to include group members. This makes it easier from a code pov. However if we are worried about group members having full admin control over jobs (i.e change the whiteboard) we can still change these controls feature by feature.