On 09/12/2012 10:24 AM, Tomas Babej wrote: > On 09/11/2012 01:14 PM, Martin Kosek wrote: >> On 09/06/2012 01:13 PM, Tomas Babej wrote: >>> On 09/05/2012 01:56 PM, Martin Kosek wrote: >>>> 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 >>> 1.) Done. >>> 2.) Corrected. >>> 3.) Fixed. >>> >>> Tomas >> 1) There are some (corner) cases where this approach still does not work: >> >> # ipa selinuxusermap-show foo >> Rule name: foo >> SELinux User: guest_u:s0 >> HBAC Rule: foo >> Enabled: TRUE >> # ipa selinuxusermap-mod foo --usercat=all --hbacrule= >> ipa: ERROR: HBAC rule and local members cannot both be set >> >> HBAC rule attribute is being deleted and user category set, so this should >> not >> be rejected. >> >> 2) There are also some styling issues (you can use pep8 tool present in >> Fedora >> to locate them on your own, e.g.: >> >> ipalib/plugins/selinuxusermap.py:247:32: E203 whitespace before ':' >> ipalib/plugins/selinuxusermap.py:247:70: E225 missing whitespace around >> operator >> ipalib/plugins/selinuxusermap.py:249:36: E221 multiple spaces before operator >> ... >> >> Martin > The corner case is fixed now and styling issues corrected as well. > > Tomas
Yup, works fine now. ACK. Pushed to master, ipa-3-0. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel