Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI

2013-12-03 Thread Petr Viktorin

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

2013-11-28 Thread Nathaniel McCallum
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

2013-11-27 Thread Petr Viktorin

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

2013-11-27 Thread Petr Viktorin

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

2013-11-27 Thread Simo Sorce
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

2013-11-27 Thread Nathaniel McCallum
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

2013-11-27 Thread Simo Sorce
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

2013-11-27 Thread Nathaniel McCallum
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

2013-11-21 Thread Nathaniel McCallum
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

2013-11-21 Thread Dmitri Pal
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

2013-11-18 Thread Petr Viktorin

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

2013-11-15 Thread Petr Viktorin

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

2013-11-11 Thread Nathaniel McCallum
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

2013-11-08 Thread Petr Viktorin

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).


The patch needs a rebase.

--
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

2013-09-25 Thread Nathaniel McCallum
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

2013-09-24 Thread Nathan Kinder

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

2013-09-20 Thread Nathaniel McCallum
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

2013-09-14 Thread Jan Cholasta

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

2013-09-13 Thread Jan Cholasta

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

2013-09-05 Thread Dmitri Pal
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

2013-09-04 Thread Nathaniel McCallum
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