[Freeipa-devel] [PATCH] 697 Add new schema to store information about permissions.

[Rob Crittenden]

Martin Kosek wrote:
> On Mon, 2011-01-31 at 22:18 -0500, Rob Crittenden wrote:
>> Rob Crittenden wrote:
>>> There are some permissions we can't display because they are stored
>>> outside of the basedn (such as the replication permissions). We are
>>> adding a new attribute to store extra information to make this clear, in
>>> this case READONLY.
>>>
>>> ticket 853
>>>
>>> rob
>>
>> I goofed on the schema, updated patch attached.
>>
>> rob
>
> NACK (but a small one)
>
> The patch is fine, I have found only 2 minor issues and a question:
>
> 1) Permission tests got broken. You may want to apply my "[PATCH] 021
> Permission rename test failing" before fixing that - so that Permission
> test suite is clean.

Ouch, ok I'll take a look.

>
> 2) In delegation.ldif: ipapermission object class is missing for
> removeentitlements and modifyentitlements (it has been added for
> addentitlements though)

This was on purpose, I should have been clearer. Patch 664 makes major 
changes to these and I'm trying to make the merge easier. I'll fix them 
up when 664 gets pushed.

>
>
> QUESTION:
> In this patch you add READONLY flag to Replica permissions. However it
> is not actually used and stays as just an informative flag. It won't
> prevent user from modifying/removing READONLY permissions.
>
> I guess enhancing permission-mod and permission-del of READONLY check
> will be a subject of another ticket?

Ok, interesting point. I considered the aci itself to be read-only. The 
only thing a user could do is rename the permission, right? I think that 
would maintain consistency so it shouldn't be a problem. It would 
probably be easy to really make these read-only but that would have a UI 
impact as well, perhaps a problematic one. I suppose if they could 
handle any read-only exceptions we'd raise that would be adequate.

rob