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

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0318.2-Add-tests-for-the-radiusproxy-plugin.patch
Type: text/x-patch
Size: 14409 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131127/a2e7bc69/attachment.bin>