[Freeipa-devel] ACI permissions UI up for review

[Adam Young]

On 12/13/2010 11:05 AM, Dmitri Pal wrote:
> Adam Young wrote:
>    
>> Dmitri,
>>
>> While I don't expect you to do the review of the patch, I would
>> appreciate at least a visual inspection of the completed UI.  Since
>> there seems to be something wrong with the install/UI right now, I've
>> posed the lates on my Fedora People page.  You should be able to see
>> it from here:
>>
>>
>>
>> http://admiyo.fedorapeople.org/ipa/static/#navigation=2&role-entity=permission&ipaserver=0&permission-facet=details&permission-pkey=modifygroupmembership
>>
>>
>>      
> I can see it.
>
>    
>>
>> Please let me know if  you can or cannot see it.  If you can, I'd like
>> to point out a couple things:
>>
>>   Selecting the object type radio button displays the attribute list in
>> a multi column form.  Selecting one of the others hides it.  Ben and I
>> decided to move it to the bottom, so that none of the other page
>> elements move.     The values are from LDAP, and may be non-intuitive
>> for someone coming to IPA from outside of that realm.  The set of
>> values displayed is from the sample data, which might need to be
>> updated.  The live site on my machine has a smaller set.  It made the
>> arbitrary decision to put 40 attributes per column, which is easy to
>> change.
>>      
> This part is fine.
>
>    
>> Biggest thing that was getting lost was the filter.  So, we moved that
>> to the top, and added a checkbox for using/not using it.  Since the
>> Filter can apply to any of the other types, v it was removed from the
>> radio button set.  The radio button et needs a 'none' option, but that
>> requires that the filter checkbox be set.  This will require a touch
>> more work.
>>      
> The screen stopped making any sense to me. I do not understand the
> structure now. Filter? What filter? Where it comes from? Why checkbox? I
> am really confused by the current layout.
>
>    
>> THe   Add button off the list page uses the same code as the details.
>> The attributes table here pushes all of the buttons way down the page,
>> but I want to leave it that way until I confirm the the updates work.
>> Once Updates are fixed (Rob has a patch up for review already)  We can
>> remove the attributes from this page.  This means that creating a
>> permission that requires attribute values will happen in two stages:
>> add, with permissions and the object, then details page, where the
>> user will select the attributes.  I can either leave the filter on, or
>> remove it from the add page as well.  I think we need the rest of the
>> info that is on there in order to successfully get through the
>> permission-add rpc.
>>      
> I have mixed feeling about this approach. What is the implication of
> having half baked rule? Is it just meaningless until the attributes are
> defined or it can potentially have a negative impact and open a security
> hole?
>    

There should be no hole.  Just the oppoosite, the rule should be 
useless, as 'write' without attributes means nothing.

But I agree, I don't really like it either.

>
>    
>> I added another option to the radio button set: targetgroup.
>>      
> I do not understand it.
>    

>    
>> This is showing up ion the ACIs that come up from permission-find.  I
>> am currently populating the select with the values from doing a
>> group-find RPC.  i've added a filter field, as we once again have the
>> possibility of having more than 100 groups, and needing to find a
>> group outside that set.  CUrrently, the query re-exectes on click of
>> the radio button, but that is not ideal.  Ben suggested that we could
>> resend the query with each keystroke, and re-populate the select box,
>> but that might generate a slew of network traffic, and slow down the
>> UI.  I won't know unless I implement, and I've lower hanging fruit to
>> pick first.
>>      
> Sorry this whole part just does not make sense to me. What is the target
> group? Where it came from?
>    

I'm wondering if this is from the groups ACI that slipped in.  Rob?
>
>    
>> According to Rob, the set of permissions can be a mix of object level
>> (add and delete) and attribute (write) although this is odd.  Thus,
>> the set of permissions is done via checkboxes.  This does mean that
>> the user can select none, and will get an error back on submit.
>>      
> I was hoping that we would be able to help to not mix things together.
> If you specify add or delete you should not be able to drill down to the
> attribute level forcing people to create explicit "write" riles
> targeting attributes.
>    

This was my understanding, and how I think it should be as well:  mixing 
the two is probably a mistake.  We'll need to fixz the undering lying 
enforcement before changing it at the web layer, though, or the UI could 
potentially receive a rule that it could not display completely.


>    
>> With Endi on PTO, Pavel is really the only one that can review this
>> code, and even he will have to take some time to learn the changes
>> that Endi and I have made since he was heads down in it.
>>
>>      
> In addition to the issues I explain above here is what I also noticed:
> 1) As we mentioned there is no "Description" in ACI. The description and
> name is the same field for ACI.
>    

Description is in the Meta data, and gets returned with ipa 
permission_show, role_show, and privilege_show

> 2) There is a label it is the name of the task group the ACI is
> associated with - it is missing
>    


It is not in the metadata.

> 3) Rest of the screen does not make much sense at all but the attribute
> part seems fine.
> 4) I do not like some of the levels on the left in the menu. It is all
> mixed up.
> 5) The Privileges, Permissions and Role Groups are jumping and changing
> places depending on your selection - this is wrong. They should just expand.
>    
They do "just expand" and contract, but we don't have any animation in 
there.  The order stays the same, but the are under each one either 
shows or hides the controls.


> 6) The hierarchy is broken for permissions
>    
What hierarchy?


> 7) When editing privileges there is unclear what the enroll button would
> do.
>    

These are auto-generated associations.  We need to filter some of them 
out, but I need to make sure we get the right subset.

> Options no include:
>
>      * Permissions Members
>      * Role Groups Members
>      * Membership in Permissions
>
> Doe not make much sense. It is really confusing if the meaning of the
> enroll button is going to change depending upon the selection made. But
> currently it does not and it completely does not make sense.
> There are more things like that related to the action panel.
>    

Yeah, we need to clean up the terms used on multiple associations pages, 
especially this one.

> In general though I like the style and where we are going with navigation.
>
>