On 11/12/2013 12:17 AM, Nathaniel McCallum wrote:
On Fri, 2013-11-08 at 13:26 +0100, Petr Viktorin wrote:
>On 09/25/2013 10:56 PM, Nathaniel McCallum wrote:
> >On Fri, 2013-09-20 at 12:38 -0400, Nathaniel McCallum wrote:
> >>On Thu, 2013-09-12 at 16:48 -0400, Nathaniel McCallum wrote:
> >>>On Thu, 2013-09-05 at 00:06 -0400, Nathaniel McCallum wrote:
> >>>>patch attached
> >>>
> >>>Update for ./makeapi attached.
> >>
> >>Version 3. This should fix all the current review issues, including the
> >>use of the referential integrity plugin. I had to make one schema change
> >>in order to make the referential integrity modification work. Note also
> >>that the command name prefix is changed from radius to radiusproxy.
> >
> >Version 4. This patch fixes my failure to increment the minor version
> >number in the VERSION file.
> >
> >Nathaniel
>
>We've since decided that we'll carry LDAP "content" updates only in
>update files, so you can leave indices.ldif & referint-conf.ldif unchanged.
>Schema, on the other hand, will still be in ldif files (and soon*only*
>in ldif files).
Fixed.

>The patch needs a rebase.
Fixed.

Also fixed: two other bugs I found when testing the above fixes. Tests
pass.

Nathaniel

Thanks for the patch!

The design page needs an update: radius-* are renamed to radiusproxy-*, several options marked in the doc as optional are mandatory.

The password can be retrieved with radiusproxy-show --all, because it is not blocked by LDAP ACIs. Is that intended?

ipatokenradiusserver is not validated. See validate_searchtimelimit in the config plugin for an example validator. You can use validate_ipaddr and validate_hostname from ipalib.util.

ipatokenusermapattribute is also not validated. Not sure if it needs to be.

The --secret CLI option does nothing (it doesn't take a value, and the secret is prompted for whether or not the option is given). It should be flagged no_option. (Or file an RFE for better Password semantics)

For the user commands, --radius takes the name on input, but a DN is output. Is that expected?

I'm attaching tests I used.

@@ -640,16 +663,29 @@ class user_mod(LDAPUpdate):
              entry_attrs['userpassword'] = ipa_generate_password(user_pwdchars)
              # save the password so it can be displayed in post_callback
              setattr(context, 'randompassword', entry_attrs['userpassword'])
-        if 'ipasshpubkey' in entry_attrs or 'ipauserauthtype' in entry_attrs:
+        if 'ipasshpubkey' in entry_attrs or 'ipauserauthtype' in entry_attrs 
or \
+           'ipatokenradiusconfiglink' in entry_attrs:
              if 'objectclass' in entry_attrs:
                  obj_classes = entry_attrs['objectclass']
              else:
                  (_dn, _entry_attrs) = ldap.get_entry(dn, ['objectclass'])

This form is deprecated. Since you don't need _dn, just do
    _entry_attrs = ldap.get_entry(dn, ['objectclass'])

                  obj_classes = entry_attrs['objectclass'] = 
_entry_attrs['objectclass']
+
              if 'ipasshpubkey' in entry_attrs and 'ipasshuser' not in 
obj_classes:
                  obj_classes.append('ipasshuser')
+
              if 'ipauserauthtype' in entry_attrs and 'ipauserauthtype' not in 
obj_classes:
                  obj_classes.append('ipauserauthtypeclass')

That should be `and 'ipauserauthtypeclass' not in obj_classes`, right? It must have slipped an earlier review.

> +
> +            if 'ipatokenradiusconfiglink' in entry_attrs:
> +                cl = entry_attrs['ipatokenradiusconfiglink']
> +                if cl:
> + if 'ipatokenradiusproxyuser' not in entry_attrs['objectclass']: > + entry_attrs['objectclass'].append('ipatokenradiusproxyuser')

Nitpick: entry_attrs['objectclass'] is stored in obj_classes, you can use that.


--
PetrĀ³

From da40f65c4805dff4def4628df189f4b7a9de413c Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Fri, 15 Nov 2013 12:26:54 +0100
Subject: [PATCH] Add tests for the radiusproxy plugin

---
 ipatests/test_xmlrpc/test_radiusproxy_plugin.py | 384 ++++++++++++++++++++++++
 1 file changed, 384 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/test_radiusproxy_plugin.py

diff --git a/ipatests/test_xmlrpc/test_radiusproxy_plugin.py b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py
new file mode 100644
index 0000000000000000000000000000000000000000..7a15a0ff0f19a3e6c0233577b2f5f16bc262f5d6
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py
@@ -0,0 +1,384 @@
+# Authors:
+#   Petr Viktorin <pvikt...@redhat.com>
+#
+# Copyright (C) 2013  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+from ipalib import errors, api, _
+from ipapython.dn import DN
+from ipatests.test_xmlrpc.xmlrpc_test import Declarative
+from ipatests.test_xmlrpc.test_user_plugin import get_user_result
+
+radius1 = u'testradius'
+radius1_fqdn = u'testradius.test'
+radius1_dn = DN(('cn=testradius'), ('cn=radiusproxy'), api.env.basedn)
+user1 = u'tuser1'
+password1 = u'very*secure123'
+
+
+class test_raduisproxy(Declarative):
+
+    cleanup_commands = [
+        ('radiusproxy_del', [radius1], {}),
+        ('user_del', [user1], {}),
+    ]
+
+    tests = [
+
+        dict(
+            desc='Try to retrieve non-existent %r' % radius1,
+            command=('radiusproxy_show', [radius1], {}),
+            expected=errors.NotFound(
+                reason=u'%s: RADIUS proxy server not found' % radius1),
+        ),
+
+
+        dict(
+            desc='Try to update non-existent %r' % radius1,
+            command=('radiusproxy_mod', [radius1], {}),
+            expected=errors.NotFound(
+                reason=_('%s: RADIUS proxy server not found') % radius1),
+        ),
+
+
+        dict(
+            desc='Try to delete non-existent %r' % radius1,
+            command=('radiusproxy_del', [radius1], {}),
+            expected=errors.NotFound(
+                reason=_('%s: RADIUS proxy server not found') % radius1),
+        ),
+
+
+        dict(
+            desc='Create %r' % radius1,
+            command=('radiusproxy_add', [radius1],
+                dict(
+                    ipatokenradiusserver=radius1_fqdn,
+                    ipatokenradiussecret=password1,
+                ),
+            ),
+            expected=dict(
+                value=radius1,
+                summary=u'Added RADIUS proxy server "%s"' % radius1,
+                result=dict(
+                    cn=[radius1],
+                    dn=radius1_dn,
+                    ipatokenradiussecret=[str(password1)],
+                    ipatokenradiusserver=[radius1_fqdn],
+                    objectclass=[u'ipatokenradiusconfiguration'],
+
+                ),
+            ),
+        ),
+
+
+        dict(
+            desc='Try to create duplicate %r' % radius1,
+            command=('radiusproxy_add', [radius1],
+                dict(
+                    ipatokenradiusserver=radius1_fqdn,
+                    ipatokenradiussecret=password1,
+                ),
+            ),
+            expected=errors.DuplicateEntry(message=_('RADIUS proxy server '
+                'with name "%s" already exists') %  radius1),
+        ),
+
+
+        dict(
+            desc='Retrieve %r' % radius1,
+            command=('radiusproxy_show', [radius1], {}),
+            expected=dict(
+                value=radius1,
+                summary=None,
+                result=dict(
+                    cn=[radius1],
+                    dn=radius1_dn,
+                    ipatokenradiusserver=[radius1_fqdn],
+                ),
+            ),
+        ),
+
+
+        dict(
+            desc='Retrieve %r with all=True' % radius1,
+            command=('radiusproxy_show', [radius1], dict(all=True)),
+            expected=dict(
+                value=radius1,
+                summary=None,
+                result=dict(
+                    cn=[radius1],
+                    dn=radius1_dn,
+                    ipatokenradiussecret=[str(password1)],
+                    ipatokenradiusserver=[radius1_fqdn],
+                    objectclass=[u'ipatokenradiusconfiguration'],
+                ),
+            ),
+        ),
+
+    ] + [
+        dict(
+            desc='Set timeout of %s to %s (valid)' % (radius1, num),
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenradiustimeout=num)),
+            expected=dict(
+                value=radius1,
+                summary=u'Modified RADIUS proxy server "%s"' % radius1,
+                result=dict(
+                    cn=[radius1],
+                    ipatokenradiusserver=[radius1_fqdn],
+                    ipatokenradiustimeout=[unicode(num)],
+                ),
+            ),
+        )
+        for num in (1, 100)
+    ] + [
+
+        dict(
+            desc='Set timeout of %s to 0 (invalid)' % radius1,
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenradiustimeout=0)),
+            expected=errors.ValidationError(
+                name='timeout', error=_('must be at least 1')),
+        ),
+
+        dict(
+            desc='Unset timeout of %s' % radius1,
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenradiustimeout=None)),
+            expected=dict(
+                value=radius1,
+                summary=u'Modified RADIUS proxy server "%s"' % radius1,
+                result=dict(
+                    cn=[radius1],
+                    ipatokenradiusserver=[radius1_fqdn],
+                ),
+            ),
+        ),
+
+    ] + [
+        dict(
+            desc='Set retries of %s to %s (valid)' % (radius1, num),
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenradiusretries=num)),
+            expected=dict(
+                value=radius1,
+                summary=u'Modified RADIUS proxy server "%s"' % radius1,
+                result=dict(
+                    cn=[radius1],
+                    ipatokenradiusserver=[radius1_fqdn],
+                    ipatokenradiusretries=[unicode(num)],
+                ),
+            ),
+        )
+        for num in (0, 4, 10)
+    ] + [
+        dict(
+            desc='Set retries of %s to %s (invalid)' % (radius1, num),
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenradiusretries=num)),
+            expected=errors.ValidationError(
+                name='retries', error=reason),
+        )
+        for num, reason in ((-1, 'must be at least 0'),
+                            (11, 'can be at most 10'),
+                            (100, 'can be at most 10'))
+    ] + [
+
+        dict(
+            desc='Unset retries of %s' % radius1,
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenradiusretries=None)),
+            expected=dict(
+                value=radius1,
+                summary=u'Modified RADIUS proxy server "%s"' % radius1,
+                result=dict(
+                    cn=[radius1],
+                    ipatokenradiusserver=[radius1_fqdn],
+                ),
+            ),
+        ),
+
+    ] + [
+        dict(
+            desc='Set server string of %s to %s (valid)' % (radius1, fqdn),
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenradiusserver=fqdn)),
+            expected=dict(
+                value=radius1,
+                summary=u'Modified RADIUS proxy server "%s"' % radius1,
+                result=dict(
+                    cn=[radius1],
+                    ipatokenradiusserver=[fqdn],
+                ),
+            ),
+        )
+        for fqdn in (radius1_fqdn + u':12345', radius1_fqdn)
+    ] + [
+        dict(
+            desc='Set server string of %s to %s (invalid)' % (radius1, fqdn),
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenradiusserver=fqdn)),
+            expected=errors.ValidationError(name='server', error=u'invalid!'),
+        )
+        for fqdn in (radius1_fqdn + u':0x5a', u'bogus')
+    ] + [
+
+        dict(
+            desc='Try to unset server string of %s' % radius1,
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenradiusserver=None)),
+            expected=errors.RequirementError(name='server'),
+        ),
+
+        dict(
+            desc='Set userattr of %s to %s (valid)' % (radius1, u'cn'),
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenusermapattribute=u'cn')),
+            expected=dict(
+                value=radius1,
+                summary=u'Modified RADIUS proxy server "%s"' % radius1,
+                result=dict(
+                    cn=[radius1],
+                    ipatokenradiusserver=[fqdn],
+                    ipatokenusermapattribute=[u'cn'],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Set userattr of %s to %s (invalid)' % (radius1, u'$%^&*'),
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenusermapattribute=u'$%^&*')),
+            expected=errors.ValidationError(name='userattr', error=u'invalid!'),
+        ),
+
+        dict(
+            desc='Unset userattr of %s' % radius1,
+            command=('radiusproxy_mod', [radius1],
+                     dict(ipatokenusermapattribute=None)),
+            expected=dict(
+                value=radius1,
+                summary=u'Modified RADIUS proxy server "%s"' % radius1,
+                result=dict(
+                    cn=[radius1],
+                    ipatokenradiusserver=[fqdn],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Set desc of %s' % radius1,
+            command=('radiusproxy_mod', [radius1],
+                     dict(description=u'a virtual radius server')),
+            expected=dict(
+                value=radius1,
+                summary=u'Modified RADIUS proxy server "%s"' % radius1,
+                result=dict(
+                    cn=[radius1],
+                    ipatokenradiusserver=[fqdn],
+                    description=[u'a virtual radius server'],
+                    ipatokenusermapattribute=[u'cn'],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Unset desc of %s' % radius1,
+            command=('radiusproxy_mod', [radius1],
+                     dict(description=None)),
+            expected=dict(
+                value=radius1,
+                summary=u'Modified RADIUS proxy server "%s"' % radius1,
+                result=dict(
+                    cn=[radius1],
+                    ipatokenradiusserver=[fqdn],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Create "%s"' % user1,
+            command=(
+                'user_add', [user1], dict(givenname=u'Test', sn=u'User1')
+            ),
+            expected=dict(
+                value=user1,
+                summary=u'Added user "%s"' % user1,
+                result=get_user_result(user1, u'Test', u'User1', 'add'),
+            ),
+        ),
+
+
+        dict(
+            desc='Set radiusconfiglink of %r' % user1,
+            command=('user_mod', [user1],
+                dict(ipatokenradiusconfiglink=radius1,)),
+            expected=dict(
+                result=get_user_result(user1, u'Test', u'User1', 'mod',
+                                       ipatokenradiusconfiglink=[radius1_dn]),
+                value=user1,
+                summary='Modified user "%s"' % user1,
+            ),
+        ),
+
+        dict(
+            desc='Retrieve %r to verify %s is output' % (radius1, user1),
+            command=('radiusproxy_show', [radius1], {}),
+            expected=dict(
+                value=radius1,
+                summary=None,
+                result=dict(
+                    cn=[radius1],
+                    dn=radius1_dn,
+                    ipatokenradiusserver=[radius1_fqdn],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Retrieve %r to verify %s is output' % (user1, radius1),
+            command=('user_show', [user1], {}),
+            expected=dict(
+                value=user1,
+                summary=None,
+                result=get_user_result(user1, u'Test', u'User1', 'show',
+                                       ipatokenradiusconfiglink=[radius1_dn]),
+            ),
+        ),
+
+        dict(
+            desc='Delete %r' % radius1,
+            command=('radiusproxy_del', [radius1], {}),
+            expected=dict(
+                value=radius1,
+                summary=u'Deleted RADIUS proxy server "%s"' % radius1,
+                result=dict(failed=u''),
+            ),
+        ),
+
+        dict(
+            desc='Retrieve %s to verify link is deleted' % user1,
+            command=('user_show', [user1], {}),
+            expected=dict(
+                value=user1,
+                summary=None,
+                result=get_user_result(user1, u'Test', u'User1', 'show'),
+            ),
+        ),
+
+    ]
-- 
1.8.3.1

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

Reply via email to