On 12/17/2012 04:08 PM, Petr Viktorin wrote:
> https://fedorahosted.org/freeipa/ticket/2482
> 
> The first two patches are rebased from what I sent back in March; the third
> fixes ACIs using targetfilter.
> 

I finally got to your patches. Generally, everything worked like charm, I have
just few minor comments:

0022:
- patch needs a rebase
- patch description is confusing, we are talking about RDN "sudocmd" and not 
"CN"

0115:
I would optimize the LDAP calls a little:
1) Use sudorule base DN as a base for the LDAP search
2) Do not call LDAP search twice, but just once and then collect the result.
Now you use 2 LDAP searches with following filters:

(&(objectClass=ipasudorule)(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))

(&(objectClass=ipasudorule)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))

We can do just one LDAP search with this filter:

(&(objectClass=ipasudorule)(|(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)))

0116:
- patch description needs amending: s/CN/SUDOCMD/

Martin

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

Reply via email to