[Freeipa-devel] [PATCH] 0015-16 Allow multiple krbprincipalnames + test

[David Kupka]

On 09/18/2014 09:42 PM, Martin Kosek wrote:
> On 09/18/2014 09:11 PM, Simo Sorce wrote:
>> On Thu, 18 Sep 2014 14:57:45 -0400
>> Rob Crittenden <rcritten at redhat.com> wrote:
>>
>>> Martin Kosek wrote:
>>>> On 09/18/2014 04:06 PM, David Kupka wrote:
>>>>> On 09/18/2014 03:44 PM, Rob Crittenden wrote:
>>>>>> David Kupka wrote:
>>>>>>> https://fedorahosted.org/freeipa/ticket/4421
>>>>>>
>>>>>> You are removing an ACI in this patch. It is always possible it
>>>>>> is no longer needed. Did you test all the client enrollment
>>>>>> scenarios?
>>>>>>
>>>>>> rob
>>>>>>
>>>>>
>>>>> As far as I'm aware I'm not removing any ACI. I'm modifying ACI so
>>>>> it is possible to add krbPrincipalName to host even when there is
>>>>> already one (or more). And adding one ACI to allow writing
>>>>> krbCanonicalName to host. But I'm still not really familiar with
>>>>> ACI so please correct me if I'm wrong.
>>>>>
>>>>
>>>> What refers to is probably the update in ACI.txt - the ACI
>>>> alternative to API.txt. David updated an ACI, not removed it.
>>>>
>>>> On that note, what is the reason for this permission change:
>>>>
>>>> -            'ipapermtargetfilter': [
>>>> -                '(objectclass=ipahost)',
>>>> -                '(!(krbprincipalname=*))',
>>>> -            ],
>>>>
>>>> ?
>>>
>>> Yes, this is exactly the change I was referring to. Permission changes
>>> within a plugin now translate automatically to ACI changes. Sorry I
>>> wasn't clearer.
>>>
>>> This ACI gets replaced with a much simpler one and I'm not 100% sure
>>> it will work with all enrollments:
>>>
>>> -aci: (targetattr = "krbprincipalname")(targetfilter =
>>> "(&(!(krbprincipalname=*))(objectclass=ipahost))")(version 3.0;acl
>>> "permission:System: Add krbPrincipalName to a Host";allow (write)
>>> groupdn = "ldap:///cn=System: Add krbPrincipalName to a
>>> Host,cn=permissions,cn=pbac,dc=ipa,dc=example";)
>>>
>>> +aci: (targetattr = "krbprincipalname")(targetfilter =
>>> "(objectclass=ipahost)")(version 3.0;acl "permission:System: Add
>>> krbPrincipalName to a Host";allow (write) groupdn =
>>> "ldap:///cn=System: Add krbPrincipalName to a
>>> Host,cn=permissions,cn=pbac,dc=ipa,dc=example";)
>>>
>>> The first one restricts writing the attribute only if it isn't already
>>> set. The second lets it be changed unconditionally.
>>
>> Yeah this is wrong indeed, the point of the ACI is to allow setting the
>> principal only when it is not already set, which is the OTP enrollment
>> case. But if krbprincipal is set then this specific permission should
>> not grant rights to change it.
>>
>> At least this was my understanding.
>>
>> Simo.
>
> Right. It seems to me we should add keep this permission intact and add
> a new permission allowing adding krbPrincipalName aliases. This would
> allow writing both krbPrincipalName and krbCanonicalName.
>
> Martin
>

Thank you all for explanation and help. I rewrote the patch so it should 
work as requested now. Also I added tests to reassure the behavior is 
correct.

-- 
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0015-2-Allow-multiple-krbprincipalnames.patch
Type: text/x-patch
Size: 7526 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140922/67446668/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0016-2-Test-Allow-multiple-krbPrincipalNames.patch
Type: text/x-patch
Size: 7348 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140922/67446668/attachment-0001.bin>