[Freeipa-devel] [PATCHES] 0578-0579 Convert Host default permissions to managed

[Martin Kosek]

On 06/19/2014 01:41 PM, Petr Viktorin wrote:
> On 06/18/2014 05:46 PM, Martin Kosek wrote:
>> On 06/11/2014 06:39 PM, Petr Viktorin wrote:
>>> Patch 0578 does the conversion
>>>
>>> Patch 0579 fixes https://fedorahosted.org/freeipa/ticket/4252 and provides
>>> permissions needed for automatic enrollment (from
>>> http://projects.theforeman.org/projects/foreman/wiki/IPASmartProxyUser)
>>
>> 1) Inconsistent casing in permission names:
>>
>> System: Add Hosts
>> System: Add krbPrincipalName to a host
>> System: Enroll a host
>> System: Manage Host SSH Public Keys
>> System: Manage host keytab
>> System: Modify Hosts
>> System: Remove Hosts
>
> Fixed
>
>> 2) This ACI does not look right, missing enrolledby:
>>
>> +        'System: Enroll a host': {
>> +            'ipapermright': {'write'},
>> +            'ipapermdefaultattr': {'objectclass'},
>>
>> When I fixed 2) via permission-mod, client enrollment with user with "Host
>> Administrators" privilege worked fine.
>
> Added
>
>> 3) I hit one issue when I open the Web UI host tab, I get "Insufficient access:
>> No such virtual command" error triggered by "cert-show" command.
>
> Virtual operations seem to be quite a can of worms.
> I've sent a separate reply for these.
>
>> We will need to add the permission "System: Read Virtual Operations" that Honza
>> is creating also to "Host Administrators" to fix that part.
>>
>>
>> 4) I ran unit tests and few missing attributes:
>> - update hosts ACI should get "macaddress" attribute
>
> Added
>
>> 5) I hit one nasty issue when running the unit tests (when my master stopped
>> working as host account was deleted) - host_is_master function in baseldap no
>> longer works as we hid cn=masters from regular users:
>>
>> def host_is_master(ldap, fqdn):
>>      """
>>      Check to see if this host is a master.
>>
>>      Raises an exception if a master, otherwise returns nothing.
>>      """
>>      master_dn = DN(('cn', fqdn), ('cn', 'masters'), ('cn', 'ipa'), ('cn',
>> 'etc'), api. env.basedn)
>>      try:
>>          ldap.get_entry(master_dn, ['objectclass'])
>>          raise errors.ValidationError(name='hostname', error=_('An IPA master
>> host      cannot be deleted or disabled'))
>>      except errors.NotFound:
>>          # Good, not a master
>>          return
>>
>> This means, that host-del on a master machine or service-del on master service
>> happily passes.
>>
>> We need to make sure this functionality is still working after the permission
>> refactoring. Should we reconsider the cn=masters tree and allow authenticated
>> users see the list of IPA servers (without digging into any other detail like
>> services) then?
>
> Nasty indeed, thanks for the catch!
>
> Sent as patch 0590, since it's a different issue than converting the host
> permissions.

Everything worked as expected, I tested both enrollments with privileged user 
and setting the OTP/class.

I have just one request (you will not like this) - before pushing please also 
fix casing for the new host permissions to match others:

+        'System: Manage host certificates': {
+        'System: Manage host enrollment password': {

When this is fixed (and ACI.txt properly updated), it is an ACK.

Martin