Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 11/28/2013 04:59 PM, Nathaniel McCallum wrote: Everything looks good to me. +1 Pushed to master: a1f32fa9369109235dba041de9c972da09d8448a -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
Everything looks good to me. +1 On Thu, 2013-11-28 at 12:18 +0100, Petr Viktorin wrote: Thanks! Just a bit of cleaning up now, sending a patch with proposed changes to speed things up. Patch needs a tiny rebase. Points I missed: - There are some unused imports. - ValidationError takes the attribute name in `name` rather than the name of the CLI option. Now the validation is too strict, a port is not accepted. Fixed. invalid! is pretty bad for an error message. I put it in as a placeholder, but I wasn't clear about that, sorry! Should non-FQDN hostnames be allowed? I agree they should not. Fixed. validate_hostname() has a check_fqdn argument, no need to do this manually. ipatokenusermapattribute is also not validated. Not sure if it needs to be. I don't think validation is really possible outside of the permitted characters for an LDAP attribute. I think if $%^* is allowed, we'll get a bug from QA soon enough. Fixed. The `sre` module is named `re` since Python 2.5. We generally output lists; this should also be a list with one element. Fixed. Attaching updated tests. A few of these tests are still failing for me, but it is not immediately obvious why. They seem to be getting answers from previous queries. I'm not sure if this is something wrong with my code or the tests. Can you take a look at it? My bad, I've used a wrong variable name. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 11/21/2013 09:54 PM, Dmitri Pal wrote: On 11/21/2013 01:34 PM, Nathaniel McCallum wrote: The password can be retrieved with radiusproxy-show --all, because it is not blocked by LDAP ACIs. Is that intended? Yes. But I'm torn as to whether or not this is a good idea. Regular users can't see radius proxy servers at all. Admins can see all attributes. It is common in radius server deployments to have a text file readable by root with the radius secret. The current LDAP policy replicates this expected behavior. It may be wise to block all reads of the secret though. I'm open to suggestions. If it is readable by admin only I would leave it as is for now and address later when we redo ACIs. CCing Simo since this is ACI-related -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
Sorry for the late review!
On 11/21/2013 07:34 PM, Nathaniel McCallum wrote:
On Fri, 2013-11-15 at 12:34 +0100, Petr Viktorin wrote:
The password can be retrieved with radiusproxy-show --all, because it is
not blocked by LDAP ACIs. Is that intended?
Yes. But I'm torn as to whether or not this is a good idea. Regular
users can't see radius proxy servers at all. Admins can see all
attributes.
It is common in radius server deployments to have a text file readable
by root with the radius secret. The current LDAP policy replicates this
expected behavior. It may be wise to block all reads of the secret
though. I'm open to suggestions.
I'm fine either way, just making sure it gets some thought.
Let's see what Simo has to say on this.
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.
Fixed.
Now the validation is too strict, a port is not accepted.
Should non-FQDN hostnames be allowed?
ipatokenusermapattribute is also not validated. Not sure if it needs to be.
I don't think validation is really possible outside of the permitted
characters for an LDAP attribute.
I think if $%^* is allowed, we'll get a bug from QA soon enough.
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)
Fixed.
For the user commands, --radius takes the name on input, but a DN is
output. Is that expected?
Fixed.
We generally output lists; this should also be a list with one element.
Attaching updated tests.
--
Petr³
From a8145b2531222604e7883b298e00929727319a5a 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/objectclasses.py | 5 +
ipatests/test_xmlrpc/test_radiusproxy_plugin.py | 384
2 files changed, 389 insertions(+)
create mode 100644 ipatests/test_xmlrpc/test_radiusproxy_plugin.py
diff --git a/ipatests/test_xmlrpc/objectclasses.py b/ipatests/test_xmlrpc/objectclasses.py
index 75ac3eb174aaa50eecfda875024b62dbdab238a5..089ee69a38b37f11de539e60b9ecacdec7a7de0b 100644
--- a/ipatests/test_xmlrpc/objectclasses.py
+++ b/ipatests/test_xmlrpc/objectclasses.py
@@ -161,3 +161,8 @@
u'nsContainer',
u'domainRelatedObject',
]
+
+radiusproxy = [
+u'ipatokenradiusconfiguration',
+u'top',
+]
diff --git a/ipatests/test_xmlrpc/test_radiusproxy_plugin.py b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py
new file mode 100644
index ..4a10b393c8a1500c1bdf354f72b962cda6ab984a
--- /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
+from ipatests.test_xmlrpc import objectclasses
+
+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(
+
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On Thu, 2013-11-21 at 15:54 -0500, Dmitri Pal wrote: On 11/21/2013 01:34 PM, Nathaniel McCallum wrote: The password can be retrieved with radiusproxy-show --all, because it is not blocked by LDAP ACIs. Is that intended? Yes. But I'm torn as to whether or not this is a good idea. Regular users can't see radius proxy servers at all. Admins can see all attributes. It is common in radius server deployments to have a text file readable by root with the radius secret. The current LDAP policy replicates this expected behavior. It may be wise to block all reads of the secret though. I'm open to suggestions. If it is readable by admin only I would leave it as is for now and address later when we redo ACIs. Is this specific to the one and only admin account or does it extend to any user in the admins group ? Looking at the current master it seem *any* user except anonymous can read secrets ? Or is there a patch I am missing ? I think this is too broad. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On Wed, 2013-11-27 at 14:34 +, Simo Sorce wrote: On Thu, 2013-11-21 at 15:54 -0500, Dmitri Pal wrote: On 11/21/2013 01:34 PM, Nathaniel McCallum wrote: The password can be retrieved with radiusproxy-show --all, because it is not blocked by LDAP ACIs. Is that intended? Yes. But I'm torn as to whether or not this is a good idea. Regular users can't see radius proxy servers at all. Admins can see all attributes. It is common in radius server deployments to have a text file readable by root with the radius secret. The current LDAP policy replicates this expected behavior. It may be wise to block all reads of the secret though. I'm open to suggestions. If it is readable by admin only I would leave it as is for now and address later when we redo ACIs. Is this specific to the one and only admin account or does it extend to any user in the admins group ? All admins. See ipatokenRadiusConfiguration in install/share/default-aci.ldif. Read access is denied to everyone except admins. The entire class is hidden from normal users. See below. Looking at the current master it seem *any* user except anonymous can read secrets ? Or is there a patch I am missing ? I think this is too broad. [root@freeipa ~]# kinit admin Password for ad...@example.com: [root@freeipa ~]# ipa radiusproxy-find - 1 RADIUS proxy server matched - RADIUS proxy server name: foo Server: foo Number of entries returned 1 [root@freeipa ~]# kinit test Password for t...@example.com: kinit: Password incorrect while getting initial credentials [root@freeipa ~]# kinit test Password for t...@example.com: [root@freeipa ~]# ipa radiusproxy-find -- 0 RADIUS proxy servers matched -- Number of entries returned 0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On Wed, 2013-11-27 at 15:12 -0500, Nathaniel McCallum wrote: On Wed, 2013-11-27 at 14:34 +, Simo Sorce wrote: On Thu, 2013-11-21 at 15:54 -0500, Dmitri Pal wrote: On 11/21/2013 01:34 PM, Nathaniel McCallum wrote: The password can be retrieved with radiusproxy-show --all, because it is not blocked by LDAP ACIs. Is that intended? Yes. But I'm torn as to whether or not this is a good idea. Regular users can't see radius proxy servers at all. Admins can see all attributes. It is common in radius server deployments to have a text file readable by root with the radius secret. The current LDAP policy replicates this expected behavior. It may be wise to block all reads of the secret though. I'm open to suggestions. If it is readable by admin only I would leave it as is for now and address later when we redo ACIs. Is this specific to the one and only admin account or does it extend to any user in the admins group ? All admins. See ipatokenRadiusConfiguration in install/share/default-aci.ldif. Read access is denied to everyone except admins. The entire class is hidden from normal users. See below. Oh I see it now, sorry, my fault in reading that ACI. I can't wait to finally straighten out our ACI system... Looking at the current master it seem *any* user except anonymous can read secrets ? Or is there a patch I am missing ? I think this is too broad. [root@freeipa ~]# kinit admin Password for ad...@example.com: [root@freeipa ~]# ipa radiusproxy-find - 1 RADIUS proxy server matched - RADIUS proxy server name: foo Server: foo Number of entries returned 1 [root@freeipa ~]# kinit test Password for t...@example.com: kinit: Password incorrect while getting initial credentials [root@freeipa ~]# kinit test Password for t...@example.com: [root@freeipa ~]# ipa radiusproxy-find -- 0 RADIUS proxy servers matched -- Number of entries returned 0 Looks good, I would still prefer to not make the password readable by default to admins, but I think we can further restrict this later once we have the new ACI system in place, which should make it easier to handle these cases. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On Wed, 2013-11-27 at 12:28 +0100, Petr Viktorin wrote:
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.
Fixed.
Now the validation is too strict, a port is not accepted.
Fixed.
Should non-FQDN hostnames be allowed?
I agree they should not. Fixed.
ipatokenusermapattribute is also not validated. Not sure if it needs to be.
I don't think validation is really possible outside of the permitted
characters for an LDAP attribute.
I think if $%^* is allowed, we'll get a bug from QA soon enough.
Fixed.
We generally output lists; this should also be a list with one element.
Fixed.
Attaching updated tests.
A few of these tests are still failing for me, but it is not immediately
obvious why. They seem to be getting answers from previous queries. I'm
not sure if this is something wrong with my code or the tests. Can you
take a look at it?
Nathaniel
From beb846295d68d2c5ce958bcf08790279761aaf58 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Mon, 11 Nov 2013 17:58:02 -0400
Subject: [PATCH] Add RADIUS proxy support to ipalib CLI
https://fedorahosted.org/freeipa/ticket/3368
---
API.txt| 95 +++--
VERSION| 2 +-
install/share/70ipaotp.ldif| 2 +-
install/updates/20-indices.update | 7 ++
install/updates/25-referint.update | 1 +
install/updates/40-otp.update | 5 ++
ipalib/constants.py| 1 +
ipalib/plugins/config.py | 2 +-
ipalib/plugins/radiusproxy.py | 166 +
ipalib/plugins/user.py | 65 +--
10 files changed, 328 insertions(+), 18 deletions(-)
create mode 100644 ipalib/plugins/radiusproxy.py
diff --git a/API.txt b/API.txt
index c29efad3382ff59e4753eff5354cba72bc1fe027..328fcf76ac6c8a8f5349e9f5631ec61bae7b3ea4 100644
--- a/API.txt
+++ b/API.txt
@@ -523,7 +523,7 @@ option: Int('ipasearchrecordslimit', attribute=True, autofill=False, cli_name='s
option: Int('ipasearchtimelimit', attribute=True, autofill=False, cli_name='searchtimelimit', minvalue=-1, multivalue=False, required=False)
option: Str('ipaselinuxusermapdefault', attribute=True, autofill=False, cli_name='ipaselinuxusermapdefault', multivalue=False, required=False)
option: Str('ipaselinuxusermaporder', attribute=True, autofill=False, cli_name='ipaselinuxusermaporder', multivalue=False, required=False)
-option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password',))
+option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password', u'radius'))
option: Str('ipauserobjectclasses', attribute=True, autofill=False, cli_name='userobjectclasses', csv=True, multivalue=True, required=False)
option: IA5Str('ipausersearchfields', attribute=True, autofill=False, cli_name='usersearch', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
@@ -2551,6 +2551,81 @@ option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: Output('value', type 'unicode', None)
+command: radiusproxy_add
+args: 1,11,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
+option: Str('addattr*', cli_name='addattr', exclude='webui')
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
+option: Int('ipatokenradiusretries', attribute=True, cli_name='retries', maxvalue=10, minvalue=0, multivalue=False, required=False)
+option: Password('ipatokenradiussecret', attribute=True, cli_name='secret', confirm=True, multivalue=False, required=True)
+option: Str('ipatokenradiusserver', attribute=True, cli_name='server', multivalue=True, required=True)
+option: Int('ipatokenradiustimeout', attribute=True, cli_name='timeout', minvalue=1, multivalue=False, required=False)
+option: Str('ipatokenusermapattribute', attribute=True, cli_name='userattr', multivalue=False, required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Str('version?', exclude='webui')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: Output('value', type 'unicode', None)
+command: radiusproxy_del
+args:
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On Fri, 2013-11-15 at 12:34 +0100, Petr Viktorin wrote:
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.
Fixed.
The password can be retrieved with radiusproxy-show --all, because it is
not blocked by LDAP ACIs. Is that intended?
Yes. But I'm torn as to whether or not this is a good idea. Regular
users can't see radius proxy servers at all. Admins can see all
attributes.
It is common in radius server deployments to have a text file readable
by root with the radius secret. The current LDAP policy replicates this
expected behavior. It may be wise to block all reads of the secret
though. I'm open to suggestions.
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.
Fixed.
ipatokenusermapattribute is also not validated. Not sure if it needs to be.
I don't think validation is really possible outside of the permitted
characters for an LDAP attribute.
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)
Fixed.
For the user commands, --radius takes the name on input, but a DN is
output. Is that expected?
Fixed.
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'])
Fixed.
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.
Fixed.
+
+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.
Fixed.
From 838b75c95d30888d72d5120a1b2e5f348deb686c Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Mon, 11 Nov 2013 17:58:02 -0400
Subject: [PATCH] Add RADIUS proxy support to ipalib CLI
https://fedorahosted.org/freeipa/ticket/3368
---
API.txt| 95 ++--
VERSION| 2 +-
install/share/70ipaotp.ldif| 2 +-
install/updates/20-indices.update | 7 ++
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 11/21/2013 01:34 PM, Nathaniel McCallum wrote: The password can be retrieved with radiusproxy-show --all, because it is not blocked by LDAP ACIs. Is that intended? Yes. But I'm torn as to whether or not this is a good idea. Regular users can't see radius proxy servers at all. Admins can see all attributes. It is common in radius server deployments to have a text file readable by root with the radius secret. The current LDAP policy replicates this expected behavior. It may be wise to block all reads of the secret though. I'm open to suggestions. If it is readable by admin only I would leave it as is for now and address later when we redo ACIs. -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 11/15/2013 12:34 PM, Petr Viktorin wrote: On 11/12/2013 12:17 AM, Nathaniel McCallum wrote: On Fri, 2013-11-08 at 13:26 +0100, Petr Viktorin wrote: 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). Heads up: this was merged. When you get a merge conflict just remove the schema update file. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
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 ..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
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
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
From aab00ba19773064a5ec43b73d5761c7145117ed8 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Mon, 11 Nov 2013 17:58:02 -0400
Subject: [PATCH] Add RADIUS proxy support to ipalib CLI
https://fedorahosted.org/freeipa/ticket/3368
---
API.txt| 95 +++--
VERSION| 2 +-
install/share/70ipaotp.ldif| 2 +-
install/updates/10-70ipaotp.update | 2 +-
install/updates/20-indices.update | 7 ++
install/updates/25-referint.update | 1 +
install/updates/40-otp.update | 5 ++
ipalib/constants.py| 1 +
ipalib/plugins/config.py | 2 +-
ipalib/plugins/radiusproxy.py | 138 +
ipalib/plugins/user.py | 50 --
11 files changed, 290 insertions(+), 15 deletions(-)
create mode 100644 ipalib/plugins/radiusproxy.py
diff --git a/API.txt b/API.txt
index cddb9d719f38dd3d89e2633148311e437aed0bc9..bee8db1fc218cd63dbb2196a38ce4086f9a4d7bb 100644
--- a/API.txt
+++ b/API.txt
@@ -514,7 +514,7 @@ option: Int('ipasearchrecordslimit', attribute=True, autofill=False, cli_name='s
option: Int('ipasearchtimelimit', attribute=True, autofill=False, cli_name='searchtimelimit', minvalue=-1, multivalue=False, required=False)
option: Str('ipaselinuxusermapdefault', attribute=True, autofill=False, cli_name='ipaselinuxusermapdefault', multivalue=False, required=False)
option: Str('ipaselinuxusermaporder', attribute=True, autofill=False, cli_name='ipaselinuxusermaporder', multivalue=False, required=False)
-option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password',))
+option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password', u'radius'))
option: Str('ipauserobjectclasses', attribute=True, autofill=False, cli_name='userobjectclasses', csv=True, multivalue=True, required=False)
option: IA5Str('ipausersearchfields', attribute=True, autofill=False, cli_name='usersearch', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
@@ -2542,6 +2542,81 @@ option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: Output('value', type 'unicode', None)
+command: radiusproxy_add
+args: 1,11,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
+option: Str('addattr*', cli_name='addattr', exclude='webui')
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
+option: Int('ipatokenradiusretries', attribute=True, cli_name='retries', maxvalue=10, minvalue=0, multivalue=False, required=False)
+option: Password('ipatokenradiussecret', attribute=True, cli_name='secret', confirm=True, multivalue=False, required=True)
+option: Str('ipatokenradiusserver', attribute=True, cli_name='server', multivalue=True, required=True)
+option: Int('ipatokenradiustimeout', attribute=True, cli_name='timeout', minvalue=1, multivalue=False, required=False)
+option: Str('ipatokenusermapattribute', attribute=True, cli_name='userattr', multivalue=False, required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Str('version?', exclude='webui')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa',
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
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
From 58d6c5672fa3b8de1e71a3c5cbec7dc90434cbac Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Wed, 4 Sep 2013 23:45:00 -0400
Subject: [PATCH] Add RADIUS proxy support to ipalib CLI
https://fedorahosted.org/freeipa/ticket/3368
---
API.txt| 95 +++--
VERSION| 2 +-
install/share/70ipaotp.ldif| 2 +-
install/share/indices.ldif | 10 +++
install/share/referint-conf.ldif | 3 +
install/updates/10-70ipaotp.update | 2 +-
install/updates/20-indices.update | 7 ++
install/updates/25-referint.update | 1 +
install/updates/40-otp.update | 5 ++
ipalib/constants.py| 1 +
ipalib/plugins/config.py | 2 +-
ipalib/plugins/radiusproxy.py | 138 +
ipalib/plugins/user.py | 44 +++-
13 files changed, 298 insertions(+), 14 deletions(-)
create mode 100644 ipalib/plugins/radiusproxy.py
diff --git a/API.txt b/API.txt
index b49493f33af0f7d0192df8318bda12df94c9567b..e662cc53e84cc3cb66e78c444e12e615bf7e3a7f 100644
--- a/API.txt
+++ b/API.txt
@@ -514,7 +514,7 @@ option: Int('ipasearchrecordslimit', attribute=True, autofill=False, cli_name='s
option: Int('ipasearchtimelimit', attribute=True, autofill=False, cli_name='searchtimelimit', minvalue=-1, multivalue=False, required=False)
option: Str('ipaselinuxusermapdefault', attribute=True, autofill=False, cli_name='ipaselinuxusermapdefault', multivalue=False, required=False)
option: Str('ipaselinuxusermaporder', attribute=True, autofill=False, cli_name='ipaselinuxusermaporder', multivalue=False, required=False)
-option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password',))
+option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password', u'radius'))
option: Str('ipauserobjectclasses', attribute=True, autofill=False, cli_name='userobjectclasses', csv=True, multivalue=True, required=False)
option: IA5Str('ipausersearchfields', attribute=True, autofill=False, cli_name='usersearch', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
@@ -2542,6 +2542,81 @@ option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: Output('value', type 'unicode', None)
+command: radiusproxy_add
+args: 1,11,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
+option: Str('addattr*', cli_name='addattr', exclude='webui')
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
+option: Int('ipatokenradiusretries', attribute=True, cli_name='retries', maxvalue=10, minvalue=0, multivalue=False, required=False)
+option: Password('ipatokenradiussecret', attribute=True, cli_name='secret', confirm=True, multivalue=False, required=True)
+option: Str('ipatokenradiusserver', attribute=True, cli_name='server', multivalue=True, required=True)
+option: Int('ipatokenradiustimeout', attribute=True, cli_name='timeout', minvalue=1, multivalue=False, required=False)
+option: Str('ipatokenusermapattribute', attribute=True, cli_name='userattr', multivalue=False, required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Str('version?', exclude='webui')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: Output('value', type 'unicode', None)
+command: radiusproxy_del
+args: 1,2,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=True, primary_key=True, query=True, required=True)
+option: Flag('continue', autofill=True, cli_name='continue', default=False)
+option: Str('version?', exclude='webui')
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 09/20/2013 09:38 AM, 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. The LDIF and update related changes look good to me. I'm not familiar with the ipalib code myself, so someone else should review that portion of your patch. Thanks, -NGK Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
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.
Nathaniel
From ddcdef7f62fe364c1081eecd9f254e8e1c0c6ee6 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Wed, 4 Sep 2013 23:45:00 -0400
Subject: [PATCH] Add RADIUS proxy support to ipalib CLI
https://fedorahosted.org/freeipa/ticket/3368
---
API.txt| 95 +++--
install/share/70ipaotp.ldif| 2 +-
install/share/indices.ldif | 10 +++
install/share/referint-conf.ldif | 3 +
install/updates/10-70ipaotp.update | 2 +-
install/updates/20-indices.update | 7 ++
install/updates/25-referint.update | 1 +
install/updates/40-otp.update | 5 ++
ipalib/constants.py| 1 +
ipalib/plugins/config.py | 2 +-
ipalib/plugins/radiusproxy.py | 138 +
ipalib/plugins/user.py | 44 +++-
12 files changed, 297 insertions(+), 13 deletions(-)
create mode 100644 ipalib/plugins/radiusproxy.py
diff --git a/API.txt b/API.txt
index b49493f33af0f7d0192df8318bda12df94c9567b..e662cc53e84cc3cb66e78c444e12e615bf7e3a7f 100644
--- a/API.txt
+++ b/API.txt
@@ -514,7 +514,7 @@ option: Int('ipasearchrecordslimit', attribute=True, autofill=False, cli_name='s
option: Int('ipasearchtimelimit', attribute=True, autofill=False, cli_name='searchtimelimit', minvalue=-1, multivalue=False, required=False)
option: Str('ipaselinuxusermapdefault', attribute=True, autofill=False, cli_name='ipaselinuxusermapdefault', multivalue=False, required=False)
option: Str('ipaselinuxusermaporder', attribute=True, autofill=False, cli_name='ipaselinuxusermaporder', multivalue=False, required=False)
-option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password',))
+option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password', u'radius'))
option: Str('ipauserobjectclasses', attribute=True, autofill=False, cli_name='userobjectclasses', csv=True, multivalue=True, required=False)
option: IA5Str('ipausersearchfields', attribute=True, autofill=False, cli_name='usersearch', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
@@ -2542,6 +2542,81 @@ option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: Output('value', type 'unicode', None)
+command: radiusproxy_add
+args: 1,11,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
+option: Str('addattr*', cli_name='addattr', exclude='webui')
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
+option: Int('ipatokenradiusretries', attribute=True, cli_name='retries', maxvalue=10, minvalue=0, multivalue=False, required=False)
+option: Password('ipatokenradiussecret', attribute=True, cli_name='secret', confirm=True, multivalue=False, required=True)
+option: Str('ipatokenradiusserver', attribute=True, cli_name='server', multivalue=True, required=True)
+option: Int('ipatokenradiustimeout', attribute=True, cli_name='timeout', minvalue=1, multivalue=False, required=False)
+option: Str('ipatokenusermapattribute', attribute=True, cli_name='userattr', multivalue=False, required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Str('version?', exclude='webui')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: Output('value', type 'unicode', None)
+command: radiusproxy_del
+args: 1,2,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=True, primary_key=True, query=True, required=True)
+option: Flag('continue', autofill=True, cli_name='continue', default=False)
+option: Str('version?', exclude='webui')
+output: Output('result', type 'dict', None)
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: Output('value', type 'unicode', None)
+command: radiusproxy_find
+args: 1,13,4
+arg:
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 13.9.2013 09:21, Jan Cholasta wrote:
Hi,
On 12.9.2013 22:48, Nathaniel McCallum wrote:
On Thu, 2013-09-05 at 00:06 -0400, Nathaniel McCallum wrote:
patch attached
Update for ./makeapi attached.
+if 'ipatokenradiusconfiglink' in entry_attrs:
+cl = entry_attrs['ipatokenradiusconfiglink']
+if not cl:
+entry_attrs['ipatokenradiususername'] = None
+if 'ipatokenradiusproxyuser' in
entry_attrs['objectclass']:
+ entry_attrs['objectclass'].remove('ipatokenradiusproxyuser')
Is there are particular reason to remove the object class? I think you
can just leave it there, that is what we do in other plugins.
+else:
+if 'ipatokenradiusproxyuser' not in
entry_attrs['objectclass']:
+ entry_attrs['objectclass'].append('ipatokenradiusproxyuser')
+
+answer = self.api.Command.radius_show(cl)
+entry_attrs['ipatokenradiusconfiglink'] =
answer['result']['dn']
Please use self.api.Object.radius.get_dn_if_exists(cl) to get the DN
instead of radius_show.
The whole code block should be added to user_add as well.
+radius = options.get('ipatokenradiusconfiglink', None)
+if radius is not None:
+answer = self.api.Command.radius_show(radius)
+filter = filter.replace('(ipatokenradiusconfiglink=%s)' %
radius,
+'(ipatokenradiusconfiglink=%s)' %
answer['result']['dn'])
Again, use get_dn_if_exists instead of radius_show to get the DN.
As for the filter processing, I think it would be safer to override
args_options_2_entry in user_find and do it in there:
def args_options_2_entry(self, *keys, **options):
if 'ipatokenradiusconfiglink' in options:
options['ipatokenradiusconfiglink'] =
self.api.Object.radius.get_dn(options['ipatokenradiusconfiglink'])
return super(user_find, self).args_options_2_entry(
... or you can do this in user_find.execute, as there already is
something similar done for the manager attribute.
Honza
BTW, I think you should configure the referential integrity plugin so
that when a radius object is deleted, all ipatokenradiusconfiglink's to
it are deleted as well.
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
Hi,
On 12.9.2013 22:48, Nathaniel McCallum wrote:
On Thu, 2013-09-05 at 00:06 -0400, Nathaniel McCallum wrote:
patch attached
Update for ./makeapi attached.
+if 'ipatokenradiusconfiglink' in entry_attrs:
+cl = entry_attrs['ipatokenradiusconfiglink']
+if not cl:
+entry_attrs['ipatokenradiususername'] = None
+if 'ipatokenradiusproxyuser' in entry_attrs['objectclass']:
+
entry_attrs['objectclass'].remove('ipatokenradiusproxyuser')
Is there are particular reason to remove the object class? I think you
can just leave it there, that is what we do in other plugins.
+else:
+if 'ipatokenradiusproxyuser' not in
entry_attrs['objectclass']:
+
entry_attrs['objectclass'].append('ipatokenradiusproxyuser')
+
+answer = self.api.Command.radius_show(cl)
+entry_attrs['ipatokenradiusconfiglink'] =
answer['result']['dn']
Please use self.api.Object.radius.get_dn_if_exists(cl) to get the DN
instead of radius_show.
The whole code block should be added to user_add as well.
+radius = options.get('ipatokenradiusconfiglink', None)
+if radius is not None:
+answer = self.api.Command.radius_show(radius)
+filter = filter.replace('(ipatokenradiusconfiglink=%s)' %
radius,
+'(ipatokenradiusconfiglink=%s)' %
answer['result']['dn'])
Again, use get_dn_if_exists instead of radius_show to get the DN.
As for the filter processing, I think it would be safer to override
args_options_2_entry in user_find and do it in there:
def args_options_2_entry(self, *keys, **options):
if 'ipatokenradiusconfiglink' in options:
options['ipatokenradiusconfiglink'] =
self.api.Object.radius.get_dn(options['ipatokenradiusconfiglink'])
return super(user_find, self).args_options_2_entry(
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 09/05/2013 12:29 AM, Nathaniel McCallum wrote: I forgot to mention that this code ignores the design page in one area: radius-show does not list the users attached to this server. How important is this? user-find --radius=MyRADIUSServer should find all the users. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel This is probably OK but then the design needs to be updated. Also I would suggest documenting that to get all users associated with a radius server you need to run the command above. -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
I forgot to mention that this code ignores the design page in one area: radius-show does not list the users attached to this server. How important is this? user-find --radius=MyRADIUSServer should find all the users. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
