On 15.3.2016 22:22, Nathaniel McCallum wrote:
On Tue, 2016-03-15 at 17:54 +0100, Martin Babinsky wrote:
On 03/15/2016 03:36 PM, Martin Babinsky wrote:


On 03/09/2016 07:06 AM, Jan Cholasta wrote:

On 8.3.2016 17:45, Martin Babinsky wrote:

On 03/08/2016 05:35 PM, Jan Cholasta wrote:

Hi,

On 8.3.2016 16:21, Martin Babinsky wrote:

https://fedorahosted.org/freeipa/ticket/5700
1) Instead of checking for utf-8 in particular, I would
prefer a more
robust approach:

try:
      qr = qrcode.QRCode()
      qr.add_data('test')
      qr.make()
      qr.print_ascii(tty=True)
except UnicodeError:
      # it is not printable
else:
      # it is printable

Now you mean the check in the _check_qrcode_capability() or the
_print_qrcode() method itself?
_check_qrcode_capability() of course.




2) There is no os.isatty() check to see if stdout is actually
a tty.

This check is performed inside both print_ascii() and
print_tty()
methods of QRCode object, but you probably mean that I should
put the
check also into _check_qrcode_capability() method, right?
Yes. If stdout is not a tty, we should at least not tty=True in
print_ascii().




Honza



Attaching updated patch. After the discussion with other developers
we
decided to just print warnings when non-UTF-8 encoding is used and
tty
width is smaller that the QR code size.



Found some minor errors in the patch, attaching updated version.

NACK

This patch has the major problem that tokens are added but then
unusable because they can't be provisioned to the devices. You need to
check if qrcode output is possible before the token is added to LDAP.

We discussed this on the IPA devel meeting and the decision was that since the otpauth URI is always displayed, a warning is sufficient when the QR code cannot be printed.

If you disagree, could you explain why the URI is not sufficient for provisioning the token?

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to