[Freeipa-devel] [PATCHES] 0536-0537 Add ACI for read-only admin attributes

[Martin Kosek]

On 04/24/2014 02:28 PM, Simo Sorce wrote:
> On Thu, 2014-04-24 at 14:17 +0200, Martin Kosek wrote:
>> On 04/24/2014 09:41 AM, Petr Viktorin wrote:
>>> On 04/23/2014 08:56 PM, Simo Sorce wrote:
>>>> On Wed, 2014-04-23 at 20:37 +0200, Petr Viktorin wrote:
>>>>> Admin access to read-only attributes such as ipaUniqueId, memberOf,
>>>>> krbPrincipalName is provided by the anonymous read ACI, which will go
>>>>> away. This patch adds a blanket read ACI for these.
>>>>> I also moved some related ACIs to 20-aci.update.
>>>>>
>>>>> Previously krbPwdHistory was also readable by admins. I don't think we
>>>>> want to include that.
>>>>> Simo, should admins be allowed to read krbExtraData?
>>>>
>>>> Probably not necessary but there is nothing secret in it either.
>>>>
>>>> Simo.
>>>
>>> OK. I'm not a fan of hiding things from the admin, so no changes to the patch
>>> are necessary here.
>>>
>>
>> 536:
>> As we are touching these ACIs, may it is a time to see the blacklist of
>> attributes that admin cannot write and check if this is still wanted:
>>
>> ipaUniqueId - OK, generated by DS plugin
>> memberOf - OK, generated by DS plugin
>> serverHostName - I did not even find a place where we manipulate it, except
>> host.py -> remove from blacklist?

What about this one?

>> enrolledBy - OK, generated by DS plugin
>> krbExtraData - OK, generated by DS plugin
>> krbPrincipalName - why can't admin change it? It is filled by framework, I
>> would not personally blacklist it
> 
> It is changed by the ipa rename plugin when the user uid change, that's
> why we prevent the admin from explicitly change it.
> 
>> krbCanonicalName - same as krbPrincipalName

Ok, in that case krbPrincipalName and krbCanonicalName should stay, right?

>> krbPrincipalAliases - same as krbPrincipalName - we need this removed if we
>> want to set aliases anyway

Allow this one?

>> krbPasswordExpiration - OK, generated by DS plugin
>> krbLastPwdChange - OK, generated by DS plugin
>> krbUPEnabled - not used, can we remove it?

What about this one?

>> krbTicketPolicyReference - why cannot admin set it?
> 
> Unclear why, probably should be able to.

We may want to remove it from blacklist then.

> 
>> krbPwdPolicyReference - why cannot admin set it?
> 
> We assign password policy based on groups now, right ?

Yup.

> 
>> krbPrincipalType - why cannot admin set it?
> 
> Unused.

So... allow this one?

> 
>> krbLastSuccessfulAuth - OK, generated by DS plugin
>> krbLastFailedAuth - OK, generated by DS plugin
>> krbLoginFailedCount - OK, generated by DS plugin
>>
>> It seems to me that some attributes can be indeed removed from the backlist
>> (and thus from the admin whitelist too).
>>
>> Besides that, the patch looked OK to me.
>>
>> 537: ACK (tests pass)


Petr, regarding the "Admin can manage any entry" update - you removed 4 related
 "remove:aci" definitions, but added just 3. Is that intentional?

Martin