[Freeipa-devel] [PATCH] 728 default roles

[Rob Crittenden]

Martin Kosek wrote:
> On Tue, 2011-02-22 at 09:22 -0500, Rob Crittenden wrote:
>> 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
>
> Good job! Its OK now. ACK
>
> Martin
>

pushed to master