On 09/03/2012 05:12 PM, Tomas Babej wrote:
> Hi,
> 
> Both selinuxusermap-add and selinuxusermap-mod commands now behave
> consistently in not allowing user/host category or user/host members
> and HBAC rule being set at the same time. Also adds a bunch of unit
> tests that check this behaviour.
> 
> https://fedorahosted.org/freeipa/ticket/2983
> 
> Tomas
> 

I found few issues with this patch:

1) Patch needs a rebase

2) Patch does not expect attributes to be set to None, i.e. to be left empty or
to be deleted, e.g.:

# ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all --hbacrule=
ipa: ERROR: HBAC rule and local members cannot both be set

# ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all
----------------------------
Added SELinux User Map "foo"
----------------------------
  Rule name: foo
  SELinux User: guest_u:s0
  User category: all
  Enabled: TRUE

# ipa selinuxusermap-mod foo --usercat= --hbacrule=
ipa: ERROR: HBAC rule and local members cannot both be set

# ipa selinuxusermap-mod foo --usercat=
-------------------------------
Modified SELinux User Map "foo"
-------------------------------
  Rule name: foo
  SELinux User: guest_u:s0
  Enabled: TRUE

# ipa selinuxusermap-mod foo --hbacrule=foo
-------------------------------
Modified SELinux User Map "foo"
-------------------------------
  Rule name: foo
  SELinux User: guest_u:s0
  HBAC Rule: foo
  Enabled: TRUE

# ipa selinuxusermap-mod foo --hbacrule= --usercat=all
ipa: ERROR: HBAC rule and local members cannot both be set

All these validation failures are not valid.

3) Additionally, I think it would be more readable and less error prone that if
instead of this blob:

+        are_local_members_to_be_set  = 'usercategory' in _entry_attrs or \
+                                       'hostcategory' in _entry_attrs or \
+                                       'memberuser' in _entry_attrs or \
+                                       'memberhost' in _entry_attrs

You would use something like that:

are_local_members_to_be_set  = any(attr in _entry_attrs
                                   for attr in ('usercategory',
                                                'hostcategory',
                                                'memberuser',
                                                'memberhost'))

Martin

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

Reply via email to