[Freeipa-devel] [PATCHES] 0562-0563 Fix internal error when global policy is not readable

[Petr Viktorin]

On 05/30/2014 11:02 AM, Petr Viktorin wrote:
> On 05/29/2014 07:13 PM, Rob Crittenden wrote:
>> Petr Viktorin wrote:
>>> When investigating this issue I became very annoyed by the star import
>>> hiding where names come from, so I did some cleanup first.
>>>
>>>
>>> In krbtpolicy, an ACIError is now raised if:
>>> - the user doesn't have permission to read any one of the ticket policy
>>>    attributes on the requested entry
>>>    (checked using attribute-level rights)
>>> - any ticket policy attribute from the default policy is not available
>>>    (either not readable, or not there at all)
>>>    (only checked if these are accessed, i.e. when the user entry doesn't
>>>     override all of the defaults, or when requesting the global policy)
>>>
>>> That means if the user is not available at all, you get a NotFound, but
>>> if global policy is not found it's assumed that it's just unreadable.
>>
>> That seems reasonable to me.
>>
>> I also noticed a typo, ddoesn't
>
> Fixed, thanks.
>
>> In the lower-level code, ldap2.py, we have some help
>> can_[read|write|etc] for managing rights. Would doing something similar
>> in baseldap be better than embedding the logic into each plugins?
>>
>> So instead of this:
>>
>>                      if rights is None:
>>                          rights = baseldap.get_effective_rights(
>>                              ldap, dn, self.obj.default_attributes)
>>                      if 'r' not in rights.get(attrname.lower(), ''):
>>                          raise errors.ACIError(
>>                              info=_('Ticket policy for %s could not be
>> read') %
>>                                  keys[-1])
>>
>> You'd have this:
>>
>>                      if not baseldap.can_read(ldap, dn, attrname):
>>                          raise errors.ACIError(
>>                              info=_('Ticket policy for %s could not be
>> read') %
>>                                  keys[-1])
>
> The second is definitely better.
>
>> This may end up fetching the rights multiple times depending on how many
>> things need to be read, so perhaps passing that in if you have it
>> already.
>
> This however means doing the get_effective_rights call anyway to get all
> the needed attrs, at which point most of the readability benefits are gone.
>
> Actually I'd like to add some good attrlevelrights handling right into
> ipaldap. Let's do this correctly rather than add a quick fix to the
> helpers just so we can use them.
> Issue here: https://fedorahosted.org/freeipa/ticket/4362

Once more, with patches

-- 
Petr³

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0562.2-krbtpolicy-plugin-Code-cleanup.patch
Type: text/x-patch
Size: 3203 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140530/33744c26/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0563.2-krbtpolicy-plugin-Fix-internal-error-when-global-pol.patch
Type: text/x-patch
Size: 3216 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140530/33744c26/attachment-0001.bin>