[Freeipa-devel] [PATCH] 552-557 Permissions v2 Web UI

[Petr Vobornik]

On 19.3.2014 17:03, Misnyovszki Adam wrote:
> On Wed, 19 Mar 2014 10:52:10 +0100
> Petr Viktorin <pviktori at redhat.com> wrote:
>
>> On 03/18/2014 04:56 PM, Petr Vobornik wrote:
>>> On 18.3.2014 15:07, Petr Viktorin wrote:
>>>> On 03/18/2014 01:09 PM, Petr Vobornik wrote:
>>>>> New revision for patch patch #557 attached.
>>>>>
>>>>> On 17.3.2014 15:22, Petr Viktorin wrote:
>>>>>> On 03/14/2014 06:47 PM, Petr Vobornik wrote:
>>>>>>> Main ACI UI changes are in patch #557. The rest are
>>>>>>> prerequisites.
>>>>>>
>>>>>> With this UI it is impossible to change from "Type-based"
>>>>>> permissions to
>>>>>> "General" ones. This seems to be remaining from the old model
>>>>>> where permissions were type/filter/subtree/targetgroup were
>>>>>> "classes" of a permission rather than co-existing as attributes.
>>>>>>
>>>>>> Rather the Target section should IMO look the same for all
>>>>>> (non-managed)
>>>>>> permissions, with the first items being:
>>>>>>       Type:    [drop-down with a None option]
>>>>>>       Subtree: [textbox that is disabled when a Type is selected]
>>>>>>
>>>>>> The Subtree should be a one-line textbox. It would be acceptable
>>>>>> if the whole DN doesn't always fit, it's the first part that's
>>>>>> important.
>>>>>>
>>>>>> Remember to only send Subtree if Type is (staying as | being set
>>>>>> to) None.
>>>>>>
>>>>>> Also, the Add dialog should use this instead of the "Define by".
>>>>>
>>>>> Done
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> With managed permissions, if I try to change both
>>>>>> included/excluded attribute list and the effective attributes, I
>>>>>> get a validation error, which is good in CLI but it doesn't work
>>>>>> well for the UI.
>>>>>>
>>>>>> I think it would be better to move "Managed permission
>>>>>> overrides" below "Target", and make it read-only. And perhaps
>>>>>> rename it to something like
>>>>>> "Attribute breakdown".
>>>>>> Managing the included/excluded lists directly is only useful for
>>>>>> upgrades with a heavily customized policy, and for upgrades you
>>>>>> need the
>>>>>> CLI anyway. Normally, having only the attribute list editable
>>>>>> should be fine.
>>>>>
>>>>> Done
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> For SYSTEM permissions (those which only have the SYSTEM flag),
>>>>>> such as 'Add Automember Rebuild Membership Task', Permissions
>>>>>> should not be editable.
p>>>>>> For old-style permissions (those without any flags), nothing is
>>>>>> editable
>>>>>> but everything should be. The attributelevelrights are missing
>>>>>> because the entry doesn't have the ipaPermissionV2 objectclass
>>>>>> yet (although it's being reported, which is "my" bug -- #4257).
>>>>>
>>>>> Fields were set to be editable if attributes level rights are
>>>>> missing.
>>>>
>>>> That solves things for normal legacy permissions, but not for the
>>>> SYSTEM ones - those should be completely read-only.
>>>>
>>>> Also, changing the Permisisons checkboxes on these permissions
>>>> doesn't mark them dirty.
>>>>
>>>> Otherwise the patches work great!
>>>>
>>>
>>> Fixed
>>>
>>> New versions of 556 and 557 attached.
>>>
>>
>> Great, thanks!
>> ACK for the functionality, I can't really judge the javascript though.
>>
>
> ACK for the code and the test, besides these two issues(don't know if it
> has to be corrected):
> 555:
> - typo in commit message(~delimeter)

fixed before push

> 557:
> - install/ui/test/aci_tests.js tab error at first row

fixed before push

>
> Greets:
> Adam
>

pushed to master:
* 1ff095333e9c5eb90b160c619d65f823f1f9f0a0 webui-static: update metadata 
files
* 80324fcb23ba6559487ba29074b608dc2bbf4945 webui: fix unit tests
* c93dd068e475a46da7d58619c83db8832ff81ba1 webui: better check for 
existing options in attributes_widgets
* d40dd17fb1658731d6df8dc805a44421f99dab38 webui: do not create <hr> 
delimiter between sections
* 4de360fd2c9ec2c67737821ddeb1c5a0b34737b1 webui: reflect enabled state 
in child widgets of a multivalued widget
* 5efcb240ce4b304ecc9f90a9bb70e1f85436d5c0 webui: change permissions UI 
to v2

-- 
Petr Vobornik