On Fri, 20 Jun 2014, Nathaniel McCallum wrote:
On Thu, 2014-06-19 at 16:30 -0400, Nathaniel McCallum wrote:
This command behaves almost exactly like otptoken-add except:
1. The new token data is written directly to a YubiKey
2. The vendor/model/serial fields are populated from the YubiKey

=== NOTE ===
1. This patch depends on the new Fedora package: python-yubico. If you
would like to help with the package review, please assign yourself here:
https://bugzilla.redhat.com/show_bug.cgi?id=1111334

New version of the patch. This one works (yay!).

1. Because of the dependency on python-yubico, is this feature something
we want in core FreeIPA? As a subpackage? Separate project altogether?
The only dependency for python-yubico is pyusb.
I'd prefer to have it integrated but have a separate dummy subpackage
that pulls in all required dependencies, like, freeipa-tools-yubico. Instead of failing when 'ipa otptoken-add-yubikey' is called, please wrap the
python-yubico import into a code that allows reporting a message back to
the user advising to install the package.

Who is a supposed user for this command? IPA command line interface isn't
usually available on enrolled machines even though underlying Python
modules are all there. Are we talking about admins or just users?


2. Should the "import yubico" statement be inside of the
otptoken_add_yubikey.forward() method to reduce server dependencies?
Here is how we do it in ipalibs/plugins/trust.py:
====================================================================
try:
   import pysss_murmur #pylint: disable=F0401
   _murmur_installed = True
except Exception, e:
   _murmur_installed = False

try:
   import pysss_nss_idmap #pylint: disable=F0401
   _nss_idmap_installed = True
except Exception, e:
   _nss_idmap_installed = False

if api.env.in_server and api.env.context in ['lite', 'server']:
   try:
       import ipaserver.dcerpc #pylint: disable=F0401
       _bindings_installed = True
   except ImportError:
       _bindings_installed = False
====================================================================

and then in the actual code we do
if not _bindings_installed:
   raise errors.NotFound(
       name=_('AD Trust setup'),
       reason=_(
           'Cannot perform join operation without Samba 4 support '
           'installed. Make sure you have installed server-trust-ad '
           'sub-package of IPA'
       )
   )

3. This code currently emits a warning from the call to otptoken-add:
WARNING: API Version number was not sent, forward compatibility not
guaranteed. Assuming server's API version, 2.89

How do I fix this?
Do not filter 'version' field in options in execute().

4. I am not sure why I have to delete the summary and value keys from
the return dictionary. It would be nice to display this summary message
just like otptoken-add.

5. Am I doing the ipatoken(vendor|model|serial) options correctly? These
aren't user settable, but we need to pass them from the yubikey
(client-side) to the server.

6. I'm not sure my use of assert or ValueError are correct. What should
I do here?

7. Considering that this is just a specialized invocation of
otptoken-add, can't we do this all on the client-side? This is why I had
originally used frontend.Local rather than frontend.Command.
You don't need to implement execute then, only forward, where you'll
forward your call to the server under otptoken_add name.

Typically in #forward we call super's forward but that is because we
in Command.forward() we  simply forward the command to the remote backend,
using the self.name. In your case we shouldn't really have a separate
command on the server under the same name, so you'll need to avoid
calling

So, it should look like this:

   def forward(self, *args, **kw):
       perform yubikey initialization
       filter out kw and args, if needed
       return self.Backend.rpcclient.forward('otptoken_add', *args, **kw)

See service_show implementation for an example.

--
/ Alexander Bokovoy

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to