I found two bugs in the ACI comparison code, one new and one old.

This fixes them and adds some more tests.

--
PetrĀ³
From 104d76aa7d9fa1480c915365ef5ec03ddf6fc6ff Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Mon, 2 Jun 2014 17:31:48 +0200
Subject: [PATCH] ipalib.aci: Fix bugs in comparison

- regression in be6edef6e48224e74344f48d25876b09cd263674:
  The __ne__ special method was named incorrectly

- regression in 1ea6def129aa459ecc3d176a3b6aebdf75de2eb7:
  The targetattr operator was never compared

Include some new comparison tests.
---
 ipalib/aci.py                    |  6 ++--
 ipatests/test_ipalib/test_aci.py | 68 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/ipalib/aci.py b/ipalib/aci.py
index cea61a9c43dda589e95e2bc555da61edbed5ce50..a55732bf19e58d8a4b36fa18bee2725d5b6584da 100755
--- a/ipalib/aci.py
+++ b/ipalib/aci.py
@@ -238,8 +238,8 @@ def isequal(self, b):
 
             if set(self.target.get('targetattr', {}).get('expression', ())) != set(b.target.get('targetattr',{}).get('expression', ())):
                 return False
-                if self.target.get('targetattr',{}).get('operator') != b.target.get('targetattr',{}).get('operator'):
-                    return False
+            if self.target.get('targetattr',{}).get('operator') != b.target.get('targetattr',{}).get('operator'):
+                return False
 
             if self.target.get('target',{}).get('expression') != b.target.get('target',{}).get('expression'):
                 return False
@@ -255,5 +255,5 @@ def isequal(self, b):
 
     __eq__ = isequal
 
-    def __neq__(self, b):
+    def __ne__(self, b):
         return not self == b
diff --git a/ipatests/test_ipalib/test_aci.py b/ipatests/test_ipalib/test_aci.py
index 6b8e64e71baef671308c8e3124e27ec4fe704a7c..40ba5e88cf7ee6d64b066ad2dc63fb7a29a607e4 100644
--- a/ipatests/test_ipalib/test_aci.py
+++ b/ipatests/test_ipalib/test_aci.py
@@ -62,7 +62,8 @@ def test_aci_parsing_7():
     check_aci_parsing('(targetattr = "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory")(version 3.0; acl "change_password"; allow (write) groupdn = "ldap:///cn=change_password,cn=taskgroups,dc=example,dc=com";;)',
         '(targetattr = "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory")(version 3.0;acl "change_password";allow (write) groupdn = "ldap:///cn=change_password,cn=taskgroups,dc=example,dc=com";;)')
 
-def test_aci_equality():
+
+def make_test_aci():
     a = ACI()
     a.name ="foo"
     a.set_target_attr(['title','givenname'], "!=")
@@ -70,6 +71,11 @@ def test_aci_equality():
     a.set_bindrule_operator("=")
     a.set_bindrule_expression("\"ldap:///cn=foo,cn=groups,cn=accounts,dc=example,dc=com\"";)
     a.permissions = ['read','write','add']
+    return a
+
+
+def test_aci_equality():
+    a = make_test_aci()
     print a
 
     b = ACI()
@@ -83,6 +89,66 @@ def test_aci_equality():
 
     assert a.isequal(b)
     assert a == b
+    assert not a != b
+
+
+def check_aci_inequality(b):
+    a = make_test_aci()
+    print a
+    print b
+
+    assert not a.isequal(b)
+    assert not a == b
+    assert a != b
+
+
+def test_aci_inequality_targetattr_expression():
+    b = make_test_aci()
+    b.set_target_attr(['givenname'], "!=")
+    check_aci_inequality(b)
+
+
+def test_aci_inequality_targetattr_op():
+    b = make_test_aci()
+    b.set_target_attr(['givenname', 'title'], "=")
+    check_aci_inequality(b)
+
+
+def test_aci_inequality_targetfilter():
+    b = make_test_aci()
+    b.set_target_filter('(objectclass=*)', "=")
+    check_aci_inequality(b)
+
+
+def test_aci_inequality_target():
+    b = make_test_aci()
+    b.set_target("ldap:///cn=bar,cn=groups,cn=accounts,dc=example,dc=com";, "=")
+    check_aci_inequality(b)
+
+
+def test_aci_inequality_bindrule_keyword():
+    b = make_test_aci()
+    b.set_bindrule_keyword("userdn")
+    check_aci_inequality(b)
+
+
+def test_aci_inequality_bindrule_op():
+    b = make_test_aci()
+    b.set_bindrule_operator("!=")
+    check_aci_inequality(b)
+
+
+def test_aci_inequality_bindrule_expression():
+    b = make_test_aci()
+    b.set_bindrule_expression("\"ldap:///cn=bar,cn=groups,cn=accounts,dc=example,dc=com\"";)
+    check_aci_inequality(b)
+
+
+def test_aci_inequality_permissions():
+    b = make_test_aci()
+    b.permissions = ['read', 'search', 'compare']
+    check_aci_inequality(b)
+
 
 def test_aci_parsing_8():
     check_aci_parsing('(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey")(version 3.0; acl "Enable Anonymous access"; allow (read, search, compare) userdn = "ldap:///anyone";;)',
-- 
1.9.0

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

Reply via email to