[Freeipa-devel] [PATCH 0058] Add the otptoken-add-yubikey command
Alexander Bokovoy
abokovoy at redhat.com
Wed Jun 25 10:35:11 UTC 2014
On Mon, 23 Jun 2014, Nathaniel McCallum wrote:
>On Mon, 2014-06-23 at 10:29 +0300, Alexander Bokovoy wrote:
>> 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?
>
>As discussed on IRC, we are currently hard-coding lots of optional
>dependencies. And breaking this apart into subpackages can be solved at
>a later point. YubiKey is also a unique case: we don't expect to be
>adding many more plugins like this.
>
>For these reasons, I have kept this as a hard dependency. To ease this
>transition, I have added python-yubico to F20 and EL6. You can help with
>the update review here:
>https://admin.fedoraproject.org/updates/python-yubico-1.2.1-3.fc20
>https://admin.fedoraproject.org/updates/python-yubico-1.2.1-3.el6
>
>> >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().
>
>I opted to not filter out version rather than hard-code it (pviktori's
>suggestion). This is based on the fact that otptoken-add-yubikey is
>tightly integrated with otptoken-add (even using some of the class
>attributes from otptoken).
>
>> >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.
>
>I still need help on this.
>
>> >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.
>
>This is no longer needed since I am doing everything in forward().
>However, listing these three as output params causes them to appear
>before the token's ID. I don't think this is the right way to output
>these. But this seems to me a framework issue.
>
>> >6. I'm not sure my use of assert or ValueError are correct. What should
>> >I do here?
>
>Still need help here.
Fixed this part.
>
>> >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.
>
>Fixed.
I'm attaching few fixups to the patch that make it proper reporting for
non-Yubikey case and also properly update VERSION.
Provisional ACK.
--
/ Alexander Bokovoy
-------------- next part --------------
>From 1c83f1943a635a4b2541addd9fc21b6155fb12d6 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Wed, 25 Jun 2014 13:32:16 +0300
Subject: [PATCH 10/10] fixup! Add the otptoken-add-yubikey command
---
ipalib/plugins/otptoken_yubikey.py | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/ipalib/plugins/otptoken_yubikey.py b/ipalib/plugins/otptoken_yubikey.py
index 30a81e5..b4f2188 100644
--- a/ipalib/plugins/otptoken_yubikey.py
+++ b/ipalib/plugins/otptoken_yubikey.py
@@ -18,6 +18,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
from ipalib import _, Str, IntEnum
+from ipalib.errors import NotFound
from ipalib.plugable import Registry
from ipalib.frontend import Command
from ipalib.plugins.otptoken import otptoken
@@ -78,7 +79,11 @@ class otptoken_add_yubikey(Command):
def forward(self, *args, **kwargs):
# Open the YubiKey
- yk = yubico.find_yubikey()
+ try:
+ yk = yubico.find_yubikey()
+ except yubico.yubikey.YubiKeyError, e:
+ raise NotFound(reason=_('No YubiKey found'))
+
assert yk.version_num() >= (2, 1)
# If no slot is specified, find the first free slot.
@@ -87,7 +92,7 @@ class otptoken_add_yubikey(Command):
used = yk.status().valid_configs()
kwargs['slot'] = sorted({1, 2}.difference(used))[0]
except IndexError:
- raise ValueError('No free YubiKey slot!')
+ raise NotFound(reason=_('No free YubiKey slot!'))
# Create the key (NOTE: the length is fixed).
key = os.urandom(20)
--
1.9.3
-------------- next part --------------
>From 3f8d3d009cc45fb28a937770bb370646a4261c39 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Wed, 25 Jun 2014 09:37:18 +0300
Subject: [PATCH 07/10] fixup! Add the otptoken-add-yubikey command
---
VERSION | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/VERSION b/VERSION
index 65e5273..f0c2db5 100644
--- a/VERSION
+++ b/VERSION
@@ -89,5 +89,5 @@ IPA_DATA_VERSION=20100614120000
# #
########################################################
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=93
-# Last change: mbasti - New record type added: DLV
+IPA_API_VERSION_MINOR=94
+# Last change: npmaccallum - otptoken-add-yubikey
--
1.9.3
More information about the Freeipa-devel
mailing list