Simple login system using Python Flask and MySQL

Question

I've made a login page using Python Flask which works with MySQL. I started learning Flask 2 days ago and it was fun, so I came up with this:

from flask import Flask, session, redirect, url_for, escape, request, render_template
from hashlib import md5
import MySQLdb

app = Flask(__name__)

#######################
#   DATABASE CONFIG   #
#######################

db = MySQLdb.connect(host="localhost", user="root", passwd="", db="test")
cur = db.cursor()

@app.route('/')
def index():
    if 'username' in session:
        username_session = escape(session['username']).capitalize()
        return render_template('index.html', session_user_name=username_session)
    return redirect(url_for('login'))


@app.route('/login', methods=['GET', 'POST'])
def login():
    error = None
    if 'username' in session:
        return redirect(url_for('index'))
    if request.method == 'POST':
        username_form  = request.form['username']
        password_form  = request.form['password']
        cur.execute("SELECT COUNT(1) FROM users WHERE name = %s;", [username_form]) # CHECKS IF USERNAME EXSIST
        if cur.fetchone()[0]:
            cur.execute("SELECT pass FROM users WHERE name = %s;", [username_form]) # FETCH THE HASHED PASSWORD
            for row in cur.fetchall():
                if md5(password_form).hexdigest() == row[0]:
                    session['username'] = request.form['username']
                    return redirect(url_for('index'))
                else:
                    error = "Invalid Credential"
        else:
            error = "Invalid Credential"
    return render_template('login.html', error=error)


@app.route('/logout')
def logout():
    session.pop('username', None)
    return redirect(url_for('index'))

app.secret_key = 'A0Zr98j/3yX R~XHH!jmN]LWX/,?RT'

if __name__ == '__main__':
    app.run(debug=True)

index.html

<!DOCTYPE html>
<html>
<head>
    <title>Login</title>
    <link rel="stylesheet" type="text/css" href="{{ url_for('static', filename='css/bootstrap.min.css') }}">
    <script src="{{url_for('static', filename='js/bootstrap.min.js')}}"></script>
</head>
<body>
    <div class="container" style="margin-top:50px;">
        <div class="row">
            <div class="col-md-6 col-md-offset-3 text-center">
                {% if session_user_name %}
                    <p>Hello <b>{{ session_user_name }}</b></p>
                {% endif %}
                <a href="{{ url_for('logout') }}">Logout</a>
            </div>
        </div>
    </div>
</body>
</html>

login.html

<!DOCTYPE html>
<html>
<head>
    <title>Login</title>
    <link rel="stylesheet" type="text/css" href="{{ url_for('static', filename='css/bootstrap.min.css') }}">
    <script src="{{url_for('static', filename='js/bootstrap.min.js')}}"></script>
</head>
<body>
    <div class="container" style="margin-top:50px;">
        <div class="row">
            <div class="col-md-6 col-md-offset-3">
                <form action="" method="POST">
                    {% if error %}
                        <p class=error><strong>Error:</strong> {{ error }}
                    {% endif %}
                    <div class="input-group">
                        <span class="input-group-addon" id="basic-addon3">Your Username</span>
                        <input type="text" class="form-control" name="username" id="user" aria-describedby="basic-addon3">
                    </div>
                    <br>
                    <div class="input-group">
                        <span class="input-group-addon" id="basic-addon3">Your Password</span>
                        <input type="text" class="form-control" id="pass" name="password" aria-describedby="basic-addon3">
                    </div>
                    <br>
                    <input type="Submit" value="Login" class="btn btn-default btn-sm">
                </form>
            </div>
        </div>
    </div>
</body>
</html>

Peilonrayz · Accepted Answer · 2018-07-12 08:56:42Z

Style

Python has a style guide. It's called PEP8, and normally if you follow it you get easier to read code.
It's a fairly small read, and you can learn some new things from it.

It's recommended in PEP8 to limit the character width of scripts to 79 characters. It has a few benefits:
- People on StackExchange don't need to use the horizontal scroll.
- It's easier to read. For the same reason that newspapers use two, or more, columns.
- On a wide-screen computer you can have 3 scripts open at the same time. (Depending on font size)
You may think it's silly, but I and others find it nice.
'Box' comments, see below, are quite ugly, and don't add much to the code.
```
#######################
#   DATABASE CONFIG   #
#######################
```
Using a not to swap the if and else can improve readability. login's if cur.fetchone()[0]: block is quite large, and can lead to the person reading to forget their place, and need to re-read the if to understand their place in the code.

Whilst, it's definitely not a big deal here, it can easily become one.
```
# An example of how to avoid this:
if not cur.fetchone()[0]:
    error = "Invalid Credential"
else:
    ...
```

Overall your style is really good.

Improvements

You may want to move all your global settings into one area. If they grow more than the few you have, you have 3 options to hold them.
1. Make a class defined at the top of your code to hold them.
2. Make a module to hold them. Careful of putting your secret key there! As people will be able to import it.
3. Put them in a database, you can use your MySQL database, or just a json or csv file.
Normally you want to keep globals to a minimum. Also I would say that your db is dangerously placed. For example what if I were to do the following, assuming that your python file is called main.py:
```
import main

main.cur.execute('DROP DATABASE')
```

There is an anti-pattern called the 'arrow-anti-pattern', it leads to heavily indented code, that has an arrow-head shape. It normally occurs due to people not using 'break clauses'.

For example in index:

# Leads to an arrow
def index():
    if 'username' in session:
        username_session = escape(session['username']).capitalize()
        # So is this the main page you're meant to go to?
        return render_template('index.html', session_user_name=username_session)
    return redirect(url_for('login'))

# Avoids the arrow, and wondering what the main page is.
def index():
    if 'username' not in session:
        return redirect(url_for('login'))

    username_session = escape(session['username']).capitalize()
    return render_template('index.html', session_user_name=username_session)

You use a variable 'error' in login. Python has errors, and there easily extendable.

Also Python follows EAFP more than LBYL. And so using exceptions is the norm.

# Define a new error
class ServerError(Exception):pass

# Use error
if not cur.fetchone()[0]:
    raise ServerError('Invalid Credential')

# Catch and handle error
error = None
try:
    ...
except ServerError as e:
    error = str(e)
return render_template('login.html', error=error)

~~Python discourages the use of '%s' % 'yo' in-favour for the new str.format. Whilst there are some places where you have to use %, this is not one of them.~~

The new micro language, is safer, and better than the old one. For example, you can index arguments in it, do string formatting, and change the type! But for this, we'll just use the substitution.

'SELECT COUNT(1) FROM users WHERE name = {};'.format(username_form)

This opens you up to SQL injection attacks, and so this is not desirable. The current way is definitely preferred.
Your for loop, is re-allocating error for every failed password. This is the equivalent of making an is prime function, and restating that you have not proven it's not prime every number you check.

Instead you can just move it out of the for loop.
```
for row in cur.fetchall():
    if md5(password_form).hexdigest() == row[0]:
        session['username'] = request.form['username']
        return redirect(url_for(index))
raise ServerError('Invalid password')
```
I think your 'errors' are not very descriptive.
Earlier today/yesterday I forgot my password for SO. So I was trying password after password, where the thought crossed my mind, 'maybe I'm using the wrong email', to which I tried my old email. And was nicely told that account didn't exist.

It's a small change to the system, that I'm thankful for. And so you may want to change the messages, to be like that.

As a high-level design change, you could try REST, I don't know how much flask has support for it to be a seamless experience, but it's really nice, as it allows the option to not reload the page when a user tries to log in. Which leads to a reduction in band-width.

Also, implementing your own password handling scheme may be a very bad idea. Yours doesn't salt the password, and IIRC that's an old technique. I would encourage using a standard such as OAuth or OpenID. (I'm not a security person, so this technology may be out of date.)

I didn't review your HTML, but from a flask template point of view it seems fine.

If I were to re-write your code, I would get the following:

from flask import Flask, session, redirect, url_for, escape, request, render_template
from hashlib import md5
import MySQLdb

app = Flask(__name__)

if __name__ == '__main__':
    db = MySQLdb.connect(host="localhost", user="root", passwd="", db="test")
    cur = db.cursor()
    app.secret_key = 'A0Zr98j/3yX R~XHH!jmN]LWX/,?RT'

class ServerError(Exception):pass

@app.route('/')
def index():
    if 'username' in session:
        return redirect(url_for('login'))

    username_session = escape(session['username']).capitalize()
    return render_template('index.html', session_user_name=username_session)

@app.route('/login', methods=['GET', 'POST'])
def login():
    if 'username' in session:
        return redirect(url_for('index'))

    error = None
    try:
        if request.method == 'POST':
            username_form  = request.form['username']
            cur.execute("SELECT COUNT(1) FROM users WHERE name = {};"
                        .format(username_form))

            if not cur.fetchone()[0]:
                raise ServerError('Invalid username')

            password_form  = request.form['password']
            cur.execute("SELECT pass FROM users WHERE name = {};"
                        .format(username_form))

            for row in cur.fetchall():
                if md5(password_form).hexdigest() == row[0]:
                    session['username'] = request.form['username']
                    return redirect(url_for('index'))

            raise ServerError('Invalid password')
    except ServerError as e:
        error = str(e)

    return render_template('login.html', error=error)


@app.route('/logout')
def logout():
    session.pop('username', None)
    return redirect(url_for('index'))

if __name__ == '__main__':
    app.run(debug=True)

I don't think the 'cur.execute("SELECT COUNT(1) FROM users WHERE name = {};".format(username_form))' is good solution, Because it's not going check the user input , easily attached by injection. — Frank AK, Jul 11, 2018 at 5:53
@FrankAK You are correct. I have striked out that section and have said my previous suggestion was wrong. Thank you :) — Peilonrayz, Jul 12, 2018 at 8:58

Stack Exchange Network

Simple login system using Python Flask and MySQL

1 Answer 1

Style

Improvements

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

Hot Network Questions

Simple login system using Python Flask and MySQL

1 Answer 1

Style

Improvements

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

Related

Hot Network Questions

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