Skip to content

Commit

Permalink
don't need QR codes
Browse files Browse the repository at this point in the history
  • Loading branch information
@dlenski
dlenski committed Dec 4, 2015
1 parent 7f29bef commit 17710cd
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 23 deletions.
2 changes: 0 additions & 2 deletions setup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,9 @@
keywords='development',
packages=['vipaccess'],
install_requires=[
'image',
'lxml',
'oath',
'PyCrypto',
'qrcode',
'requests',
],
entry_points={
Expand Down
4 changes: 0 additions & 4 deletions tests/test_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,6 @@ def test_generate_otp_uri():
assert urlparse.parse_qs(generated_uri.params) == urlparse.parse_qs(expected_uri.params)
assert urlparse.parse_qs(generated_uri.query) == urlparse.parse_qs(expected_uri.query)

def test_generate_qr_code():
test_uri = 'otpauth://totp/VIP%20Access:VSST26070843?secret=LJYWKRGZO5TV2IQSD434O5RWELYBGXDJ&issuer=Symantec'
assert generate_qr_code(test_uri)

def test_check_token_detects_valid_token():
test_request = generate_request()
test_response = requests.post(PROVISIONING_URL, data=test_request)
Expand Down
17 changes: 0 additions & 17 deletions vipaccess/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
except ImportError:
import urllib

import qrcode
import requests
from Crypto.Cipher import AES
from Crypto.Random import random
Expand Down Expand Up @@ -238,19 +237,6 @@ def generate_otp_uri(token_id, secret):

return 'otpauth://%(otp_type)s/%(app_name)s:%(account_name)s?%(parameters)s' % token_parameters

def generate_qr_code(uri):
'''Generate a QR code from the OTP URI.'''
qr = qrcode.QRCode(
version=1,
error_correction=qrcode.constants.ERROR_CORRECT_L,
box_size=10,
border=4
)
qr.add_data(uri)
qr.make(fit=True)
im = qr.make_image()
return im

def check_token(token_id, secret):
'''Check the validity of the generated token.'''
otp = totp(binascii.b2a_hex(secret).decode('utf-8'))
Expand Down Expand Up @@ -284,7 +270,4 @@ def main():
print(otp_uri)
print("BE AWARE that this new credential expires on this date: " + otp_token['expiry'])

image = generate_qr_code(otp_uri)
image.show()

return True

3 comments on commit 17710cd

@rombert
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't raise an issue on a fork, so commenting here ...

First of all, thanks for adding the option to generate mobile tokens, it came in really handy!

I would like to ask you to reconsider dropping QR code support. For importing the generated token into a mobile application ( Google Authenticator, FreeOTP, etc ) this is by far the simplest way. I mucked around with the generated token a lot trying to import it into a mobile app with no luck, but using a QR code was dead easy.

I am not familiar with python, but perhaps you can make the image and qrcode dependencies optional and then add a new CLI flag ( off by default ) which shows the image?

@dlenski
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'm glad it's useful for you! Switching from the "desktop" to "mobile" tokens (whatever that actually means) was just a one-line change to the source code: 912166e

I can re-add the QR code support if you like. My thinking was that users could just use a separate command-line tool like qrencode to actually convert the URL to a QR code, e.g.:

echo http://foo.bar.baz | qrencode -o- | display

My own personal use case for Symantec VIP Access doesn't involve mobile devices at all, so I didn't realize the QR codes were widely used :-)

@rombert
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had no idea about qrencode or how to pipe the URL that this tool generates to it :-)

For me it would be enough if this would be specified in the README close to the bullet point that says that qr code generation is no longer supported.

Please sign in to comment.