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

[Petr Viktorin]

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.
>
> 1. The handling of the 'key' option is insecure. It should probably be
> treated like a password (hidden from logs, etc). However, in this case,
> it is binary, so I'm not quite sure how to do that. Passing it as a
> command line option may be nice for scripting, but is potentially a
> security problem if it ends up in bash.history. It would also be nice if
> the encoding were base32 instead of base64, since nearly all the OTP
> tools use this encoding.
>
> 2. The 'key' option also appears in otp-find. I'd like to suppress this.
> How?
>
> 3. I had to make the 'id' option optional to make the uuid
> autogeneration work in otp-add. However, this has the side-effect that
> 'id' is now optional in all the other commands. This is particularly bad
> in the case of otp-del, where calling this command with no ID
> transparently removes all tokens. How can I make this optional for
> otp-add but required for all other commands?
>
> 4. otp-import is not implemented. I spent a few hours looking and I
> didn't find any otp tool that actually uses this xml format for
> exporting. Should we implement this now or wait until someone can
> actually export data to us?
>
> 5. otp-del happily deletes the last token for a user. How can I find out
> the dn of the user executing the command? Also, what is the right
> exception to throw in pre_callback()?
>
> 6. user-show does not list the associated tokens for this user. Do we
> care? It is a single search: otp-find --owner npmccallum.
>
> 7. otp-add only prints the qr code if the --qrcode option is specified.
> This is for two reasons. First, and most importantly, the qr code
> doesn't fit on a standard 24x80 terminal. I wanted to avoid dumping
> garbage on people's screens by default. Second, you may not always want
> the qr code output (like for a hard token or manual code entry).
>
> Nathaniel

For the record:
This patch was superseded by Nathaniel's patch 0024.


-- 
Petr³