Python Flask webserver with small features

Question

With the latest of my projects, I started using Visual Studio Code, which allows for Python linting and also checking for poor practices, such as using over 100 characters in a single line. As you will see, my variable names sometimes get long, because I want to maintain readable variable names. Just for information, I also handle logins and sessions with a Flask session cookie. Below are the various files. Also worth mentioning is that I use a local Redis server for Windows to test the application.

Python version: 3.5.2 32 bit

server.py

"""Main server"""
import json
import os
import gamble
import authentication
import gcaptchacheck
from flask import Flask, session, redirect, url_for, request, render_template
from flask_compress import Compress
from flask_sslify import SSLify
from store import REDISDB

if not REDISDB.get('READY'):
    REDISDB.set('users', json.dumps(dict()))
    REDISDB.set('READY', 'READY')

_STYSHEET = '/static/styles.css?version=1'
_SKEY = '//removed for security'
LAST_TIME_UPDATE = 0

APP = Flask(__name__)
Compress(APP)
APP.secret_key = '//removed for security'
SSLIFY = SSLify(APP, permanent=True)

@APP.route('/')
def index():
    """The home page."""
    if 'user' in session:
        user = session['user']
        userdb = json.loads(REDISDB.get('users').decode('UTF-8'))
        if user in userdb:
            return render_template('index.html', button='Profile', logged_in=True,
                                text=user, stylesheet=_STYSHEET)
    return render_template('index.html', text='Not logged in.', stylesheet=_STYSHEET)

@APP.route('/login/create/new/')
def usercreationpage():
    """Create user page"""
    emsg = request.args.get('emsg')
    xusername = request.args.get('xusername')
    prefill = True
    if not xusername:
        prefill = False
    return render_template('createuser.html', stylesheet=_STYSHEET, username=xusername,
                        errormsg=emsg, pfill=prefill)

@APP.route('/login/create/submit/')
def createuser():
    """Make user"""
    iusername = request.args.get('username')
    pwa = request.args.get('password')
    if pwa == request.args.get('password2'):
        #optimization, multiple IFs
        userdb = json.loads(REDISDB.get('users').decode('UTF-8'))
        if not userdb.get(iusername):
            if not request.headers.getlist("X-Forwarded-For"):
                client_ip = request.remote_addr
            else:
                client_ip = request.headers.getlist("X-Forwarded-For")[0]
            #verify captcha
            captcha_response = gcaptchacheck.checkcaptcha(request.args.get('g-recaptcha-response'),
                                                        client_ip, _SKEY)
            if captcha_response:
                uncheck = authentication.validateusername(iusername)
                if not uncheck['success']:
                    return redirect(url_for('usercreationpage', xusername=iusername,
                                            emsg=uncheck['reason']))
                pwcheck = authentication.validatepassword(pwa)
                if not pwcheck['success']:
                    return redirect(url_for('usercreationpage', xusername=iusername,
                                            emsg=pwcheck['reason']))
                #if things have gotten this far all good, make the account
                userdata = authentication.createuserset(pwa)
                #[salt, hashed pwd + salt, empty string for shared secret]
                userdb.update({iusername: userdata})
                REDISDB.set('users', json.dumps(userdb))
                return redirect(url_for('index'))
            if not captcha_response:
                return redirect(url_for('usercreationpage', xusername=iusername,
                                        emsg='Bad captcha'))
        return redirect(url_for('usercreationpage', xusername=iusername, emsg='Username taken'))
    return redirect(url_for('usercreationpage', xusername=iusername, emsg='Passwords do not match'))

@APP.route('/login/existing/')
def loginpage():
    """The login page"""
    emsg = request.args.get('emsg')
    xusername = request.args.get('xusername')
    prefill = True
    if not xusername:
        prefill = False
    return render_template('loginpage.html', errormsg=emsg, username=xusername,
                        stylesheet=_STYSHEET, pfill=prefill)

@APP.route('/login/existing/submit/')
def verifylogin():
    """Verifies a login"""
    iusername = request.args.get('username')
    ipassword = request.args.get('password')
    userdb = json.loads(REDISDB.get('users').decode('UTF-8'))
    result = authentication.authenticateuser(iusername, ipassword, userdb)
    if result:
        session['user'] = iusername
        return redirect(url_for('index'))
    return redirect(url_for('loginpage', xusername=iusername, emsg='Invalid login'))

@APP.route('/logout/')
def logout():
    """Log out"""
    session.pop('user', None)
    return redirect(url_for('index'))

@APP.route('/profile/')
def profilepage():
    """User's profile page"""
    if 'user' in session:
        user = session['user']
        userdb = json.loads(REDISDB.get('users').decode('UTF-8'))
        if user in userdb:
            userinfo = userdb.get(user)
            return render_template('profile.html', username=user, stylesheet=_STYSHEET)
    return redirect(url_for('index'))

@APP.route('/double/')
def double():
    """Double or nothing page"""
    if 'user' in session:
        user = session['user']
        userdb = json.loads(REDISDB.get('users').decode('UTF-8'))
        if user in userdb:
            userinfo = userdb.get(user)
            return render_template('double.html', coin_amt=userinfo[2], text='Gamble a coin.',
                                stylesheet=_STYSHEET)
    return redirect(url_for('index'))

@APP.route('/dodouble/')
def dodouble():
    """Double"""
    if 'user' in session:
        user = session['user']
        userdb = json.loads(REDISDB.get('users').decode('UTF-8'))
        if user in userdb:
            userinfo = userdb.get(user)
            message, success, change = gamble.double(userinfo, request.args.get('gamount'))
            if success:
                userinfo[2] += change
                userdb.update({user: userinfo})
                REDISDB.set('users', json.dumps(userdb))
            return message
    return "-"

@APP.route('/getcoins/')
def getcoins():
    """Get coins"""
    if 'user' in session:
        user = session['user']
        userdb = json.loads(REDISDB.get('users').decode('UTF-8'))
        if user in userdb:
            userinfo = userdb.get(user)
            return str(userinfo[2])
    return "-"

@APP.route('/admin/')
def admin():
    """Admin"""
    if 'user' in session:
        if session['user'] == '//removed for privacy':
            userdb = json.loads(REDISDB.get('users').decode('UTF-8'))
            return render_template('admin.html', userdict=userdb)
    return redirect(url_for('index'))

@APP.route('/setmoney/')
def setmoney():
    """Set money"""
    if 'user' in session:
        if session['user'] == '//removed for privacy':
            money = request.args.get('money')
            user = request.args.get('user')
            userdb = json.loads(REDISDB.get('users').decode('UTF-8'))
            userdb[user][2] = int(money)
            REDISDB.set('users', json.dumps(userdb))
            return redirect(url_for('admin'))
    return redirect(url_for('index'))

if __name__ == '__main__':
    try:
        APP.run('0.0.0.0', 80, True)
    except PermissionError:
        HPORT = int(os.environ.get('PORT', 17995))
        APP.run('0.0.0.0', HPORT, False)

authentication.py

"""Authentication module for authentication"""
import hashlib
import random

ALLOWED_CHARS = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*()'
ALLOWED_UCHAR = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890'

def generatesalt(length, charset):
    """Makes a salt"""
    returnstring = ''
    while len(returnstring) < length:
        returnstring += charset[random.randint(0, len(charset) - 1)]
    return returnstring

def validatepassword(password):
    """Checks if password is good"""
    if len(password) > 1:
        if checkletter(password, ALLOWED_CHARS):
            return {'success': True}
        return {'success': False,
                'reason': 'Password must be alphanumeric or with symbols.'}
    return {'success': False, 'reason': 'Password must be at least 2 characters.'}

def validateusername(username):
    """Checks if username is good"""
    if len(username) > 3:
        if checkletter(username, ALLOWED_UCHAR):
            return {'success': True}
        return {'success': False,
                'reason': 'Username must be alphanumeric.'}
    return {'success': False, 'reason': 'Username must be at least 4 characters.'}

def checkletter(text, allowed_set):
    """Checks letters"""
    for letter in text:
        if not letter in allowed_set:
            return False
    return True

def createuserset(password):
    """Create user information"""
    salt = generatesalt(16, ALLOWED_UCHAR)
    hashsum = hashlib.sha256((password + salt).encode('UTF-8')).hexdigest()
    return [salt, hashsum, 1000]

def authenticateuser(username, password, dbs):
    """Checks whether a username matches to a password"""
    print(dbs)
    userinformation = dbs.get(username)
    if userinformation:
        print(userinformation)
        if (hashlib.sha256((password + userinformation[0])
                        .encode('UTF-8')).hexdigest() == userinformation[1]):
            return True
    return False

gcaptchacheck.py

"""Gcaptchacheck"""
import requests

def checkcaptcha(clientresponse, clientip, secret):
    """Verify"""
    url = 'https://www.google.com/recaptcha/api/siteverify'
    argdict = {'secret':secret, 'response':clientresponse, 'remoteip':clientip}
    print(argdict)
    google_response = requests.post(url, argdict)
    response_dict = google_response.json()
    return response_dict['success']

store.py

"""Store module"""
import os
import urllib
import redis

DEBUG = False

if DEBUG:
    REDISDB = redis.Redis(host='', port=6379, db=0, password='')
else:
    URL = urllib.parse.urlparse(os.environ.get('REDISTOGO_URL', 'redis://localhost:6379'))
    REDISDB = redis.Redis(host=URL.hostname, port=URL.port, db=0, password=URL.password)

*DEBUG variable is true when in deployment!

gamble.py

"""Gamble"""
import random

def double(userinfo, amt):
    """Double"""
    if not amt:
        return 'Bad request', False, 0
    try:
        amt = int(amt)
    except ValueError:
        return 'Bad request', False, 0
    if (userinfo[2] - amt >= 0) and (amt > 0):
        #allow
        result = random.randint(0, 1)
        if result:
            return 'Won ' + str(amt) + ' coins.', True, amt
        return 'Lost ' + str(amt) + ' coins.', True, 0 - amt
    return 'Insufficient balance', False, 0

This is also one of the first times I have worked with an application where other dependencies written by me are stored alongside in a folder.

The main issues I have are:

1. Am I handling login sessions appropriately? Should I manually store cookies b64encoded in front of some more encryption?

2. My variable names are often quite long, and sometimes if / else ... statements become too long from multiple indents - how do I avoid that?

   My variable names get rather long just by themselves. An example could be gcaptchacheck.checkcaptcha or authentication.validateusername.

3. I am simply storing user passwords as a SHA256 hash, along with a salt (securely!)... from what I know that's a good way to go, but are there better practices?

4. My app sends asynchrnous requests using some JavaScript in the HTML such as /dodouble/?gamount=xxx and /getcoins/. Is there a better way that I can use?

5. I am storing my files alongside each other in a folder. Is there a better way in terms of organisation or convention? I am considering something like gunicorn to enhance performance. Is there a better alternative? I am developing on Windows, after all.

   I control the stylesheets because otherwise when I feel like updating the stylesheets, I have to edit the HTML for all the files. This method gives me easy way to manage.

6. At the end you will see except PermissionError, which is because deploying to Heroku will disallow you to bind to port 80, raising PermissionError. This method to automate this part seems to be relatively simple. Any better ways to do this? Heroku has an environment variable for Django apps to figure out whether they're in a development environment or deployment environment. Is it OK if I snatch that quick fix, or is there an unspoken "sin" in doing that?

I'm not great at Python, so further recommendations would be appreciated!

It also seems like nobody is interested or there is too much information to digest. This link to the website should help people interpret what I'm doing a bit better.

Edit: Sorry, the link has been deprecated.

Answer 1

1. Looks ok to me. You would need a Story that better articulates the actors and the threat model before you could judge whether alternate storage of cookies would adequately address the described threat.

2. Your variable names are too short. Add at least one character, so we have e.g. validate_username. Descriptive identifiers are a good thing, keep using them.

You are already using parens to deal nicely with long boolean expressions, e.g.:

    if (hashlib.sha256((password + userinformation[0])
                    .encode('UTF-8')).hexdigest() == userinformation[1]):

Consider writing long expressions in this way:

    if (a > b
        and c > d
        and e > f):

In checkletter() you wrote if not letter in allowed_set:, but flake8 would explain to you that the usual idiom is if leter not in.... No biggie.

Functions like validatepassword() would be a little clearer if you threw in the occasional else. Doesn't change how the program runs, it's just for folks reading it.

3. Using sha256 should be fine. Though I did notice a literal assignment to APP.secret_key, suggesting the secret is checked into source control. It is usual to instead read credentials from a separately maintained config file on the side.

4. Your API calls are fine.

5. Using gunicorn or flask is pretty mainstream, you should be fine with low volume traffic. When scaling up, you will likely frontend with reverse proxy from nginx or varnish, for cacheable static assets.

6. Consulting a Heroku environment variable is perfectly fine, that's what it's there for.