Re: [Freeipa-devel] [PATCHES] 0022, 0115-0116 Make Sudo commands case-sensitive

2013-02-20 Thread Martin Kosek
On 12/17/2012 04:08 PM, Petr Viktorin wrote:
 https://fedorahosted.org/freeipa/ticket/2482
 
 The first two patches are rebased from what I sent back in March; the third
 fixes ACIs using targetfilter.
 

I finally got to your patches. Generally, everything worked like charm, I have
just few minor comments:

0022:
- patch needs a rebase
- patch description is confusing, we are talking about RDN sudocmd and not 
CN

0115:
I would optimize the LDAP calls a little:
1) Use sudorule base DN as a base for the LDAP search
2) Do not call LDAP search twice, but just once and then collect the result.
Now you use 2 LDAP searches with following filters:

((objectClass=ipasudorule)(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))

((objectClass=ipasudorule)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))

We can do just one LDAP search with this filter:

((objectClass=ipasudorule)(|(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)))

0116:
- patch description needs amending: s/CN/SUDOCMD/

Martin

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


Re: [Freeipa-devel] [PATCHES] 0022, 0115-0116 Make Sudo commands case-sensitive

2013-02-20 Thread Petr Viktorin

On 02/20/2013 12:46 PM, Martin Kosek wrote:

On 12/17/2012 04:08 PM, Petr Viktorin wrote:

https://fedorahosted.org/freeipa/ticket/2482

The first two patches are rebased from what I sent back in March; the third
fixes ACIs using targetfilter.



I finally got to your patches. Generally, everything worked like charm, I have
just few minor comments:

0022:
- patch needs a rebase
- patch description is confusing, we are talking about RDN sudocmd and not 
CN

0115:
I would optimize the LDAP calls a little:
1) Use sudorule base DN as a base for the LDAP search
2) Do not call LDAP search twice, but just once and then collect the result.
Now you use 2 LDAP searches with following filters:

((objectClass=ipasudorule)(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))

((objectClass=ipasudorule)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))

We can do just one LDAP search with this filter:

((objectClass=ipasudorule)(|(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)))

0116:
- patch description needs amending: s/CN/SUDOCMD/

Martin



Thanks. Attaching updated patches.

--
PetrĀ³
From 87485f70aca0932cfb53a63c91842f762602e9b2 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 8 Mar 2012 07:55:00 -0500
Subject: [PATCH] Use ipauniqueid for the RDN of sudo commands

Since sudo commands are case-sensitive, we can't use 'sudocmd'
as the RDN.

Tests for case-sensitive behavior included

https://fedorahosted.org/freeipa/ticket/2482
---
 ipalib/plugins/sudocmd.py |1 +
 tests/test_xmlrpc/test_sudocmd_plugin.py  |   82 +---
 tests/test_xmlrpc/test_sudocmdgroup_plugin.py |   85 +
 tests/test_xmlrpc/xmlrpc_test.py  |   13 +++-
 4 files changed, 153 insertions(+), 28 deletions(-)

diff --git a/ipalib/plugins/sudocmd.py b/ipalib/plugins/sudocmd.py
index 42068edea3c51804be9ee5919934462afbee578f..f27a58cadd6e6abc16611621387f26125737bf78 100644
--- a/ipalib/plugins/sudocmd.py
+++ b/ipalib/plugins/sudocmd.py
@@ -62,6 +62,7 @@ class sudocmd(LDAPObject):
 'memberof': ['sudocmdgroup'],
 }
 uuid_attribute = 'ipauniqueid'
+rdn_attribute = 'ipauniqueid'
 label = _('Sudo Commands')
 label_singular = _('Sudo Command')
 
diff --git a/tests/test_xmlrpc/test_sudocmd_plugin.py b/tests/test_xmlrpc/test_sudocmd_plugin.py
index 75b6bbccbb47cc03f4408b791c687f3880bb934a..0ea8e10b8144f1b16527f2786be24de75eb6 100644
--- a/tests/test_xmlrpc/test_sudocmd_plugin.py
+++ b/tests/test_xmlrpc/test_sudocmd_plugin.py
@@ -21,18 +21,20 @@
 Test the `ipalib/plugins/sudocmd.py` module.
 
 
-from ipalib import api, errors
-from tests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid
+from ipalib import errors
+from tests.test_xmlrpc.xmlrpc_test import (Declarative, fuzzy_sudocmddn,
+fuzzy_uuid)
 from tests.test_xmlrpc import objectclasses
-from ipapython.dn import DN
 
 sudocmd1 = u'/usr/bin/sudotestcmd1'
+sudocmd1_camelcase = u'/usr/bin/sudoTestCmd1'
 
 
 class test_sudocmd(Declarative):
 
 cleanup_commands = [
 ('sudocmd_del', [sudocmd1], {}),
+('sudocmd_del', [sudocmd1_camelcase], {}),
 ]
 
 tests = [
@@ -72,16 +74,35 @@ class test_sudocmd(Declarative):
 value=sudocmd1,
 summary=u'Added Sudo Command %s' % sudocmd1,
 result=dict(
-dn=DN(('sudocmd',sudocmd1),('cn','sudocmds'),('cn','sudo'),
-  api.env.basedn),
+dn=fuzzy_sudocmddn,
 sudocmd=[sudocmd1],
 description=[u'Test sudo command 1'],
 objectclass=objectclasses.sudocmd,
 ipauniqueid=[fuzzy_uuid],
 ),
 ),
 ),
 
+dict(
+desc='Create %r' % sudocmd1_camelcase,
+command=('sudocmd_add', [sudocmd1_camelcase],
+dict(
+description=u'Test sudo command 2',
+),
+),
+expected=dict(
+value=sudocmd1_camelcase,
+summary=u'Added Sudo Command %s' % sudocmd1_camelcase,
+result=dict(
+dn=fuzzy_sudocmddn,
+sudocmd=[sudocmd1_camelcase],
+description=[u'Test sudo command 2'],
+objectclass=objectclasses.sudocmd,
+ipauniqueid=[fuzzy_uuid],
+),
+),
+),
+
 
 dict(
 desc='Try to create duplicate %r' % sudocmd1,
@@ -94,16 +115,26 @@ class test_sudocmd(Declarative):
 u'name %s already exists' % sudocmd1),
 ),
 
+dict(
+desc='Try to create duplicate %r' % sudocmd1_camelcase,
+command=('sudocmd_add', 

Re: [Freeipa-devel] [PATCHES] 0022, 0115-0116 Make Sudo commands case-sensitive

2013-02-20 Thread Martin Kosek
On 02/20/2013 05:00 PM, Petr Viktorin wrote:
 On 02/20/2013 12:46 PM, Martin Kosek wrote:
 On 12/17/2012 04:08 PM, Petr Viktorin wrote:
 https://fedorahosted.org/freeipa/ticket/2482

 The first two patches are rebased from what I sent back in March; the third
 fixes ACIs using targetfilter.


 I finally got to your patches. Generally, everything worked like charm, I 
 have
 just few minor comments:

 0022:
 - patch needs a rebase
 - patch description is confusing, we are talking about RDN sudocmd and not
 CN

 0115:
 I would optimize the LDAP calls a little:
 1) Use sudorule base DN as a base for the LDAP search
 2) Do not call LDAP search twice, but just once and then collect the result.
 Now you use 2 LDAP searches with following filters:

 ((objectClass=ipasudorule)(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))


 ((objectClass=ipasudorule)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))


 We can do just one LDAP search with this filter:

 ((objectClass=ipasudorule)(|(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)))


 0116:
 - patch description needs amending: s/CN/SUDOCMD/

 Martin

 
 Thanks. Attaching updated patches.
 

ACK. Pushed to master, ipa-3-1.

Martin

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