[Freeipa-devel] [PATCH] 728 default roles

[Rob Crittenden]

Martin Kosek wrote:
> On Tue, 2011-02-22 at 13:14 +0100, Jan Zelený wrote:
>> Rob Crittenden<rcritten at 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 at 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

Thanks for the careful review. Updated patch attached.

rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-728-4-roles.patch
Type: application/mbox
Size: 21249 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110222/5d96f629/attachment.mbox>