On 05/30/2014 03:33 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
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

Sure, I'm ok with waiting and doing it right.

Once more, with patches

ACK x2

Thanks!
Pushed to master: 63a2147ac2bca82c710a6ffd025d4dbd8f1b3449


--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to