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

[Petr Viktorin]

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

-- 
Petr³