[Freeipa-devel] [PATCH 0017] Add OTP support to ipalib CLI

[Petr Vobornik]

On 09/05/2013 06:25 AM, Nathaniel McCallum wrote:
> This patch has a few problems that I'd like some help with. There are a
> few notes here as well.
>
snip

Some additional findings:

1. Inconsistency: 'ipatokenowner' in command output should be normalized 
the same way as 'manager' in user plugin or 'seealso' in selinuxusermap. 
See _normalize_manager and _convert_manager methods. Question for all: 
Why don't we have general methods for such task?

2. Inconsistency: IntEnum doesn't convert input value as Int does. It 
should also allow to specify int in a form of unicode string (IMO).

3. IDK how OTP matching works internally, so the following might not be 
an issue, it just looks suspicious to me: I'm talking about handling of 
default values for ipatokenotpalgorithm, ipatokenotpdigits and 
ipatokentotptimestep.
- Defaults are hardcoded in otp_add.pre_callback and the same in auth.c.
- when values are not supplied, OTP token configuration uri is created 
with the defaults.
- the values are not saved to LDAP
What will happen when these defaults will change (ie. when we want to 
use more secure hashing algorithm)? I assume that OTP daemon will use 
its defaults if there are no values in LDAP. After such change the 
defaults are different than the values the token was configured with so 
the evaluation process will fail.

Should we save the values to LDAP? Or can we be sure that the defaults 
won't change? Or am I completely wrong?

4. When I pass incorrectly formatted values for options 
ipatokennotbefore and ipatokennotafter
i will get an error message saying:
"ipatokenNotBefore: value #0 invalid per syntax: Invalid syntax."
This message doesn't tell me what's is the correct format nor there is 
any description.
-- 
Petr Vobornik