3
\$\begingroup\$

Google has an Authenticator app that, given a secret and the time, shows a 6-digit code. The code cycles every 30 seconds. I wrote an implementation in Python 3 (can be run in Python 2 with some changes) that optionally generates a qrcode for a new user and/or just shows the current code displayed in the Authenticator app.

import hmac, base64, struct, hashlib, time, pyqrcode, random, sys, os

def get_hotp_token(secret, intervals_no):
    key = base64.b32decode(secret)
    msg = struct.pack(">Q", intervals_no)
    h = hmac.new(key, msg, hashlib.sha1).digest()
    o = h[19] & 15
    h = (struct.unpack(">I", h[o:o+4])[0] & 0x7fffffff) % 1000000
    return h

def get_totp_token(secret):
    return get_hotp_token(secret, intervals_no=int(time.time())//30)

# This works whenever I run it, but I'm not 100% sure it conforms to RFC standards
# Also, I'm not sure if the secret is every allowed to not be 16 bytes
def generate_secret(length=16):
    random_secret = ""
    characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567"
    for _ in range(length):
        random_secret += random.choice(characters)
    return random_secret

# Pads secret with zeroes and separates with a space
def format_secret(secret, to_length=6):
    padded = '0'*(6-len(str(secret)))+str(secret)
    return padded[:3] + ' ' + padded[3:]

# Sets up a user given a label, username, and generates/takes a secret
# Is there a better way to do the "y/n: " stuff?
def setup_user():
    label = input("Label: ").replace(' ', '%20')
    user = input("User: ").replace(' ', '%20')
    secret = input("Auto-generate secret? y/n: ")

    if secret.lower() == "y":
        secret = generate_secret()
    else: 
        secret = input("Secret: ")

    return label, user, secret

secret = "DID NOT SET SECRET"

# Again, is there a better way to do the "y/n: "?
if input("Setup user? y/n: ").lower() == "y":
    label, user, secret = setup_user()

    if input("Show qr-code? y/n: ").lower().strip() == "y":
        # I'm unsure if this URL breaks when certain characters are in the label or username
        qr_url = "otpauth://totp/%s:%s?secret=%s&issuer=%s" % (label, user, secret, label)
        url = pyqrcode.create(qr_url)
        os.system('cls' if os.name == 'nt' else 'clear')
        # Is it possible to make the qrcode smaller when printed in Terminal/Console?
        print(url.terminal(quiet_zone=1))
else:
    secret = input("Secret: ")
    os.system('cls' if os.name == 'nt' else 'clear')

print("User secret: ", secret)

sys.stdout.write("\r%s" % format_secret(get_totp_token(secret)))
sys.stdout.flush()
# This next line is my code to time-sync the timer. I'm not sure if it actually works though. 
# I think it might have to do with using modular arithmetic on a float
time.sleep(1 - time.time() % 1)

while True: 
    # These next couple of lines format a progress bar for how many seconds are left until a new code.
    seconds = int(30 - time.time() % 30)
    lines = "-" * int(seconds)# / 3)
    blocks = "█" * (30 - len(lines))
    progress = "|" + blocks + lines + "|"

    sys.stdout.write("\r%s" % format_secret(get_totp_token(secret)) + "\t" + progress)
    sys.stdout.flush()

    time.sleep(1)

This uses the pyqrcode library, which does not ship standard with Python.

I noticed that sometimes, the code does not change exactly when it does in the Authenticator app. This may be a problem with my time-syncing code.

What are your thoughts on quality of code, usability, pythonic-ness, and correctness?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Code quality

  • Each import should have its own, seperate line:

    import hmac
    import base64
    import struct
    import hashlib
    import time
    import pyqrcode
    import random
    import sys
    import os
    
  • Top-level functions should be seperated using two blank lines:

    def foo():
        foobar()
    
    
    def bar():
        foobar()
    
  • A lot of your comments should actually be docstrings.

Security

  • Don't use the random module for security purposes. Skimming through the documentation, red banners warn you not to use it for security purposes and instead to use the secrets module. Note that secrets is only part of the standard library in Python 3.6 and upwards. If you need to run this on other versions of Python, which is quite likely, make use of random.SystemRandom (or os.urandom() directly).
  • Use getpass.getpass() instead of plain input() for gathering secrets like passwords or private codes. [getpass documentation]

General recommendations

  • Since pyqrcode isn't part of the standard library, you may want to wrap import pyqrcode in a try:/except: statement:

    try:
        import pyqrcode
    except ModuleNotFoundError:
        print("Couldn't import pyqrcode.")
        sys.exit(1)
    
  • If you plan to release this as a module, it's a very good idea to add documentation (docstrings, as well as a README for quick reference), so others understand how the code works and how they should use it.

\$\endgroup\$
3
  • \$\begingroup\$ What do you think about the y/n: stuff? Do you think there is a better way to do this –– to get some boolean value? \$\endgroup\$
    – Daniel
    Jul 16, 2017 at 21:28
  • \$\begingroup\$ Your solution is actually pretty good. There's a couple of ways to handle these types of situations. You could try making it a bit more explicit by defining behavior in an enum (options = {"y": foo(), "n": bar()}). If you don't want to use that, you may want to check if input("Setup user? [y/n]: ").lower().strip() in ("yes", "y"):, so users have a bit more space. \$\endgroup\$
    – Daniel
    Jul 16, 2017 at 21:35
  • \$\begingroup\$ The most important issue I think is with my time-syncing code: I am not sure, but does using % on a float cause rounding/accuracy problems? \$\endgroup\$
    – Daniel
    Jul 16, 2017 at 21:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.