Martin Kosek wrote:
On Tue, 2012-01-24 at 10:08 -0500, Rob Crittenden wrote:
Martin Kosek wrote:
On Mon, 2012-01-23 at 12:20 -0500, Rob Crittenden wrote:
Martin Kosek wrote:
On Tue, 2012-01-17 at 17:59 -0500, Rob Crittenden wrote:
When deleting an HBAC rule we need to ensure that an SELinux user map
isn't pointing at it. The search for this didn't work well at all.

This patch corrects the search and makes it more specific.

I also tested that it works with the --continue flag of hbacrule-del.

The ticket has instructions on testing.

rob

Works fine. There is just one part that is IMO too complicated:

+            hbacrule = options['seealso']
+            kw = dict(cn=hbacrule, all=True)
                _entries = api.Command.hbacrule_find(None, **kw)['result']
                del options['seealso']
-            if _entries:
-                options['seealso'] = _entries[0]['dn']
+            found = False
+            # look for an exact match. The search may return partial
+            # matches.
+            for entry in _entries:
+                if entry['cn'][0] == hbacrule:
+                    found = True
+                    options['seealso'] = entry['dn']
+            if not found:
+                return dict(count=0, result=[], truncated=False)

I think hbacrule_find(None, cn=HBACRULE) should not return partial
matches, but just the exact match (tried with hbacrule-find
--name=HBACRULE). Then the loop over entries wouldn't be needed.

Couldn't we simply call hbacrule_show since we want just one HBAC rule
with a known primary key?

Martin


hbacrule_show would need to be modified to take a dn, that would be a
way to fix this.

rob

Not sure I see the problem with hbacrule_show. I tested this piece of
code and it worked fine:

selinuxusermap_find:
...
          if 'seealso' in options:
              hbacrule = options['seealso']

              try:
                  hbac = api.Command['hbacrule_show'](hbacrule,
all=True)['result']
                  dn = hbac['dn']
              except errors.NotFound:
                  return dict(count=0, result=[], truncated=False)
              options['seealso'] = dn
...

Martin


Ok, I misunderstood your point. Yes, this is vastly better. Updated
patch attached.

rob

ACK if you change

if 'seealso' in options:

to:

if options.get('seealso'):

so that the following case is fixed:

# ipa selinuxusermap-find --hbacrule=
ipa: ERROR: 'cn' is required

Martin


Fixed, pushed to master and ipa-2-2

rob

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

Reply via email to