ipalib.aci: Port to Python 3

- Don't encode under Python 3, where shlex would choke on bytes
- Sort the attrs dictionary in export_to_string, so the tests are
  deterministic. (The iteration order of dicts was always unspecified,
  but was always the same in practice under CPython 2.)

Reviewed-By: Tomas Babej <tbabej@redhat.com>

3 files changed, 16 insertions, 14 deletions

diff --git a/ipalib/aci.py b/ipalib/aci.py
index 687ac6357..a76435f0a 100755
--- a/ipalib/aci.py
+++ b/ipalib/aci.py
@@ -75,16 +75,16 @@ class ACI:
         """Output a Directory Server-compatible ACI string"""
         self.validate()
         aci = ""
-        for t in self.target:
-            op = self.target[t]['operator']
-            if type(self.target[t]['expression']) in (tuple, list):
+        for t, v in sorted(self.target.items()):
+            op = v['operator']
+            if type(v['expression']) in (tuple, list):
                 target = ""
-                for l in self.target[t]['expression']:
+                for l in v['expression']:
                     target = target + l + " || "
                 target = target[:-4]
                 aci = aci + "(%s %s \"%s\")" % (t, op, target)
             else:
-                aci = aci + "(%s %s \"%s\")" % (t, op, self.target[t]['expression'])
+                aci = aci + "(%s %s \"%s\")" % (t, op, v['expression'])
         aci = aci + "(version 3.0;acl \"%s\";%s (%s) %s %s \"%s\"" % (self.name, self.action, ",".join(self.permissions), self.bindrule['keyword'], self.bindrule['operator'], self.bindrule['expression']) + ";)"
         return aci
 
@@ -97,7 +97,9 @@ class ACI:
         return s
 
     def _parse_target(self, aci):
-        lexer = shlex.shlex(aci.encode('utf-8'))
+        if six.PY2:
+            aci = aci.encode('utf-8')
+        lexer = shlex.shlex(aci)
         lexer.wordchars = lexer.wordchars + "."
 
         l = []
diff --git a/ipatests/test_ipalib/test_aci.py b/ipatests/test_ipalib/test_aci.py
index 794d4e82b..fc484988f 100644
--- a/ipatests/test_ipalib/test_aci.py
+++ b/ipatests/test_ipalib/test_aci.py
@@ -41,7 +41,7 @@ def test_aci_parsing_1_with_aci_keyword():
 
 def test_aci_parsing_2():
     check_aci_parsing('(target="ldap:///uid=bjensen,dc=example,dc=com")(targetattr=*) (version 3.0;acl "aci1";allow (write) userdn="ldap:///self";)',
-        '(targetattr = "*")(target = "ldap:///uid=bjensen,dc=example,dc=com")(version 3.0;acl "aci1";allow (write) userdn = "ldap:///self";)')
+        '(target = "ldap:///uid=bjensen,dc=example,dc=com")(targetattr = "*")(version 3.0;acl "aci1";allow (write) userdn = "ldap:///self";)')
 
 def test_aci_parsing_3():
     check_aci_parsing(' (targetattr = "givenName || sn || cn || displayName || title || initials || loginShell || gecos || homePhone || mobile || pager || facsimileTelephoneNumber || telephoneNumber || street || roomNumber || l || st || postalCode || manager || secretary || description || carLicense || labeledURI || inetUserHTTPURL || seeAlso || employeeType  || businessCategory || ou")(version 3.0;acl "Self service";allow (write) userdn = "ldap:///self";)',
@@ -53,11 +53,11 @@ def test_aci_parsing_4():
 
 def test_aci_parsing_5():
     check_aci_parsing('(targetattr=member)(target="ldap:///cn=ipausers,cn=groups,cn=accounts,dc=example,dc=com")(version 3.0;acl "add_user_to_default_group";allow (write) groupdn="ldap:///cn=add_user_to_default_group,cn=taskgroups,dc=example,dc=com";)',
-        '(targetattr = "member")(target = "ldap:///cn=ipausers,cn=groups,cn=accounts,dc=example,dc=com")(version 3.0;acl "add_user_to_default_group";allow (write) groupdn = "ldap:///cn=add_user_to_default_group,cn=taskgroups,dc=example,dc=com";)')
+        '(target = "ldap:///cn=ipausers,cn=groups,cn=accounts,dc=example,dc=com")(targetattr = "member")(version 3.0;acl "add_user_to_default_group";allow (write) groupdn = "ldap:///cn=add_user_to_default_group,cn=taskgroups,dc=example,dc=com";)')
 
 def test_aci_parsing_6():
     check_aci_parsing('(targetattr!=member)(targe="ldap:///cn=ipausers,cn=groups,cn=accounts,dc=example,dc=com")(version 3.0;acl "add_user_to_default_group";allow (write) groupdn="ldap:///cn=add_user_to_default_group,cn=taskgroups,dc=example,dc=com";)',
-        '(targetattr != "member")(targe = "ldap:///cn=ipausers,cn=groups,cn=accounts,dc=example,dc=com")(version 3.0;acl "add_user_to_default_group";allow (write) groupdn = "ldap:///cn=add_user_to_default_group,cn=taskgroups,dc=example,dc=com";)')
+        '(targe = "ldap:///cn=ipausers,cn=groups,cn=accounts,dc=example,dc=com")(targetattr != "member")(version 3.0;acl "add_user_to_default_group";allow (write) groupdn = "ldap:///cn=add_user_to_default_group,cn=taskgroups,dc=example,dc=com";)')
 
 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";)',
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index 88401190a..b9c8d1b98 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -1453,8 +1453,8 @@ class test_permission(Declarative):
 
         verify_permission_aci(
             permission1, api.env.basedn,
-            '(targetattr = "sn")' +
             '(target = "ldap:///%s")' % DN('cn=editors', groups_dn) +
+            '(targetattr = "sn")' +
             '(version 3.0;acl "permission:%s";' % permission1 +
             'allow (write) groupdn = "ldap:///%s";)' % permission1_dn,
         ),
@@ -1623,8 +1623,8 @@ class test_permission_rollback(Declarative):
 
         verify_permission_aci(
             permission1, users_dn,
-            '(targetattr = "sn")' +
             '(target = "ldap:///%s")' % DN(('uid', 'admin'), users_dn) +
+            '(targetattr = "sn")' +
             '(version 3.0;acl "permission:%s";' % permission1 +
             'allow (write) groupdn = "ldap:///%s";)' % permission1_dn,
         ),
@@ -1964,8 +1964,8 @@ class test_permission_sync_attributes(Declarative):
 
         verify_permission_aci(
             permission1, groups_dn,
-            '(targetattr = "sn")' +
             '(target = "ldap:///%s")' % DN(('cn', 'editors'), groups_dn) +
+            '(targetattr = "sn")' +
             '(targetfilter = "%s")' % group_filter +
             '(version 3.0;acl "permission:%s";' % permission1 +
             'allow (write) groupdn = "ldap:///%s";)' % permission1_dn,
@@ -2000,8 +2000,8 @@ class test_permission_sync_attributes(Declarative):
 
         verify_permission_aci(
             permission1, groups_dn,
-            '(targetattr = "sn")' +
             '(target = "ldap:///%s")' % DN(('cn', 'editors'), groups_dn) +
+            '(targetattr = "sn")' +
             '(targetfilter = "(&(cn=blabla)%s)")' % group_filter +
             '(version 3.0;acl "permission:%s";' % permission1 +
             'allow (write) groupdn = "ldap:///%s";)' % permission1_dn,
@@ -2235,8 +2235,8 @@ class test_permission_sync_nice(Declarative):
 
         verify_permission_aci(
             permission1, groups_dn,
-            '(targetattr = "sn")' +
             '(target = "ldap:///%s")' % DN(('cn', 'editors'), groups_dn) +
+            '(targetattr = "sn")' +
             '(targetfilter = "%s")' % group_filter +
             '(version 3.0;acl "permission:%s";' % permission1 +
             'allow (write) groupdn = "ldap:///%s";)' % permission1_dn,