On Tue, 2011-02-22 at 13:14 +0100, Jan Zelený wrote: > Rob Crittenden <rcrit...@redhat.com> wrote: > > Jakub Hrozek wrote: > > > On Mon, Feb 21, 2011 at 10:11:38AM -0500, Rob Crittenden wrote: > > >> Rob Crittenden wrote: > > >>> Jakub Hrozek wrote: > > >>>> -----BEGIN PGP SIGNED MESSAGE----- > > >>>> Hash: SHA1 > > >>>> > > >>>> On 02/17/2011 04:35 AM, Rob Crittenden wrote: > > >>>>> Add default roles and permissions for HBAC, SUDO and pw policy > > >>>>> > > >>>>> Created some default roles as examples. In doing so I realized that > > >>>>> we were completely missing default rules for HBAC, SUDO and password > > >>>>> policy so I added those as well. > > >>>>> > > >>>>> I ran into a problem when the updater has a default record and an add > > >>>>> at the same time, it should handle it better now. > > >>>>> > > >>>>> ticket 585 > > >>>>> > > >>>>> rob > > >>>> > > >>>> I'm not sure about the HBAC rules ACIs. They are specified as: > > >>>> > > >>>> 'target = "ldap:///cn=*,cn=hbac,$SUFFIX"' > > >>>> > > >>>> while HBAC rules' DN is: > > >>>> > > >>>> 'ipauniqueid=*,cn=hbac,$SUFFIX'. > > >>>> > > >>>> But HBAC rules do have a cn: attribute, so maybe the ACIs would work? > > >>> > > >>> No, you're right, this is wrong. I'll fix it up and resubmit. > > >>> > > >>>> The patch also needs rebasing on top of recent changes to > > >>>> install/updates/Makefile.am > > >>>> > > >>>> Other than that, looks OK to me. > > >>>> > > >>>> btw when I was reviewing this patch, I noticed we add a "DNS > > >>>> Administrators" privilege in dns.ldif. Would it make sense to add DNS > > >>>> administration to "Security Architect" (replication management) and > > >>>> "IT Specialist" (hosts management)? > > >>> > > >>> The DNS stuff is added only if DNS is enabled on the server so I can't > > >>> add them by default. > > >>> > > >>> rob > > >> > > >> Updated patch. > > >> > > >> rob > > > > > > Interdiff looks fine, but I'm not able to apply the patch (not even > > > 3-way merge), can you rebase? > > > > done > > The patch now applies ok (just one whitespace warning), ack > > Jan > > _______________________________________________ > Freeipa-devel mailing list > Freeipa-devel@redhat.com > https://www.redhat.com/mailman/listinfo/freeipa-devel
I have to NACK this. I have found some issues in the new LDAP records: 1) A wrong groupdn for the following ACI in 40-delegation.update: add:aci: '(target = "ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX")(version 3.0;acl "permission:Add SUDO rule";allow (add) groupdn = "ldap:///cn=Add SUDOrule,cn=permissions,cn=pbac,$SUFFIX";)' It should be dap:///cn=Add SUDO rule,cn=permissions,cn=pbac,$SUFFIX 2) Another wrong target for few ACIs: ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX is used instead of ldap:///ipaUniqueID=*,cn=sudorules,cn=sudo,$SUFFIX 3) Missing Description for the following new privileges: Write IPA Configuration Modify Users and Reset passwords Modify Group membership Remainder looks good. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel