Issue #53 - Debug and refine login and signup functions

kylelhk commented 4 months ago

Change Summary

Both forms – AJAX error messages for client-side validations are now displayed properly under navbar upon clicking the submit button.
Login form – Given the simplicity of its server-side input validation, instead of form-level error message, an AJAX message for invalid input is displayed under the navbar.
Removed redundancies in routes.py and forms.py as the server-side validation is now solely conducted in routes.py. A lab facilitator has advised on Teams that WTForms validators and forms.py are not mandatory. To play safe, I am also seeking confirmation from UC on the group project channel about the necessity of WTForms validators and forms.py. Upon confirmation, we may make further adjustments on the code files, e.g. removing forms.py or refactoring the code to implement WTForms validators instead.
Due to time limitation, the time-based locking and exponential backoff mechanism for the login function is abandoned at this stage – removed last_failed_login and failed_login_attempts from user.py accordingly.

Since I can't implement the formatting configurations mentioned in PR #50 and PR #62 is yet to be merged with main, in this PR there are many changes tracked for auto-formatting (mainly in login.js, in which the changes are minimal and stated below). For ease of reference and on top of the above, key changes on the code are summarised below:

In login.js, applied displayAjaxMessage() instead of displayFlashMessage()to display error messages for invalid inputs, removed displayFlashMessage().
In routes.py, removed redundancies in handle_login_ajax(), handle_signup_ajax(), and validate_username() regarding server-side validations.
In forms.py, removed redundant validators and functions.
In login.js and routes.py, standardised the error messages across client-side and server-side validations.

Manual testings have been conducted on login and signup functions, client-side and server-side input validations, and access control. No anomaly was noted.

Change Form

[x] The pull request title includes issue number and description.
[x] The work done is summarised above.
[x] The code is commented and well formatted.
[x] Testing has been performed and documented above.

Other Information

N/A

iheathers commented 4 months ago

The validation process appears overly complex, suggesting possible over-engineering. It may be more efficient to handle all the logic on the server side using WTForms.

Are all functions still necessary if you want to retain them on the client side?

iheathers commented 4 months ago

It's too late to make change i guess. But, you could do something like this on jinja and handle all the thing. (in case you are not aware). You could still add few input validation on frontend. but that's not even required because if the validation is not right, that message can be thrown under each field. You won't even have to make any ajax request at all. Ajax request would have been a good call if it were an API. Ours is not.

The tutorial by miguel has all the necessary steps required for this. https://blog.miguelgrinberg.com/post/the-flask-mega-tutorial-part-v-user-logins

at the moment, there is mix of logic in both server side and client side. This can introduce hard to find bug, makes it difficult to maintain.

For idea,

`forms.py`

Create a new file forms.py to define the forms.

from flask_wtf import FlaskForm
from wtforms import StringField, PasswordField, BooleanField, SubmitField
from wtforms.validators import DataRequired, Length, Email, EqualTo, ValidationError
from app import User

class LoginForm(FlaskForm):
    username = StringField('Username', validators=[DataRequired(), Length(min=2, max=20)])
    password = PasswordField('Password', validators=[DataRequired()])
    remember_me = BooleanField('Remember Me')
    submit = SubmitField('Login')

class SignupForm(FlaskForm):
    username = StringField('Username', validators=[DataRequired(), Length(min=2, max=20)])
    email = StringField('Email', validators=[DataRequired(), Email()])
    password = PasswordField('Password', validators=[DataRequired(), Length(min=8)])
    confirm_password = PasswordField('Confirm Password', validators=[DataRequired(), EqualTo('password')])
    submit = SubmitField('Sign Up')

    def validate_username(self, username):
        user = User.query.filter_by(username=username.data).first()
        if user:
            raise ValidationError('That username is already taken. Please choose a different one.')

    def validate_email(self, email):
        user = User.query.filter_by(email=email.data).first()
        if user:
            raise ValidationError('That email is already registered. Please choose a different one.')

`app.py`

Update app.py to use the forms and handle the form submissions.

from flask import Flask, render_template, request, redirect, url_for, flash
from flask_sqlalchemy import SQLAlchemy
from flask_login import LoginManager, login_user, current_user, logout_user, login_required, UserMixin
from werkzeug.security import generate_password_hash, check_password_hash
from datetime import datetime
import pytz
from forms import LoginForm, SignupForm

app = Flask(__name__)
app.config['SECRET_KEY'] = 'your_secret_key'
app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///site.db'
db = SQLAlchemy(app)
login_manager = LoginManager(app)
login_manager.login_view = 'login'

timezone = pytz.timezone("Australia/Perth")

class User(UserMixin, db.Model):
    id = db.Column(db.Integer, primary_key=True)
    username = db.Column(db.String(64), index=True, unique=True)
    email = db.Column(db.String(120), index=True, unique=True)
    password_hash = db.Column(db.String(128))
    last_login = db.Column(db.DateTime, default=lambda: datetime.now(timezone))

    def set_password(self, password):
        self.password_hash = generate_password_hash(password)

    def check_password(self, password):
        return check_password_hash(self.password_hash, password)

@login_manager.user_loader
def load_user(user_id):
    return User.query.get(int(user_id))

@app.route('/')
def home():
    return render_template('home.html')

@app.route('/login', methods=['GET', 'POST'])
def login():
    if current_user.is_authenticated:
        return redirect(url_for('home'))
    form = LoginForm()
    if form.validate_on_submit():
        user = User.query.filter_by(username=form.username.data).first()
        if user and user.check_password(form.password.data):
            login_user(user, remember=form.remember_me.data)
            return redirect(url_for('home'))
        else:
            flash('Invalid username or password', 'danger')
    return render_template('login.html', form=form)

@app.route('/signup', methods=['GET', 'POST'])
def signup():
    if current_user.is_authenticated:
        return redirect(url_for('home'))
    form = SignupForm()
    if form.validate_on_submit():
        new_user = User(username=form.username.data, email=form.email.data)
        new_user.set_password(form.password.data)
        db.session.add(new_user)
        db.session.commit()
        flash('Registration successful, please log in', 'success')
        return redirect(url_for('login'))
    return render_template('signup.html', form=form)

@app.route('/logout')
@login_required
def logout():
    logout_user()
    return redirect(url_for('home'))

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

`login.html`

Update the login.html to use the form.

{% extends "base.html" %}
{% block title %}Login{% endblock %}
{% block content %}
<div class="container">
    <h2>Login</h2>
    <form method="POST" action="{{ url_for('login') }}">
        {{ form.hidden_tag() }}
        <div class="form-group">
            {{ form.username.label(class="form-label") }}
            {{ form.username(class="form-control") }}
            {% if form.username.errors %}
            <div class="invalid-feedback">
                {% for error in form.username.errors %}
                <span>{{ error }}</span>
                {% endfor %}
            </div>
            {% endif %}
        </div>
        <div class="form-group">
            {{ form.password.label(class="form-label") }}
            {{ form.password(class="form-control") }}
            {% if form.password.errors %}
            <div class="invalid-feedback">
                {% for error in form.password.errors %}
                <span>{{ error }}</span>
                {% endfor %}
            </div>
            {% endif %}
        </div>
        <div class="form-group">
            {{ form.remember_me() }}
            {{ form.remember_me.label() }}
        </div>
        <button type="submit" class="btn btn-primary">{{ form.submit.label }}</button>
    </form>
</div>
{% endblock %}

`signup.html`

Update the signup.html to use the form.

{% extends "base.html" %}
{% block title %}Sign Up{% endblock %}
{% block content %}
<div class="container">
    <h2>Sign Up</h2>
    <form method="POST" action="{{ url_for('signup') }}">
        {{ form.hidden_tag() }}
        <div class="form-group">
            {{ form.username.label(class="form-label") }}
            {{ form.username(class="form-control") }}
            {% if form.username.errors %}
            <div class="invalid-feedback">
                {% for error in form.username.errors %}
                <span>{{ error }}</span>
                {% endfor %}
            </div>
            {% endif %}
        </div>
        <div class="form-group">
            {{ form.email.label(class="form-label") }}
            {{ form.email(class="form-control") }}
            {% if form.email.errors %}
            <div class="invalid-feedback">
                {% for error in form.email.errors %}
                <span>{{ error }}</span>
                {% endfor %}
            </div>
            {% endif %}
        </div>
        <div class="form-group">
            {{ form.password.label(class="form-label") }}
            {{ form.password(class="form-control") }}
            {% if form.password.errors %}
            <div class="invalid-feedback">
                {% for error in form.password.errors %}
                <span>{{ error }}</span>
                {% endfor %}
            </div>
            {% endif %}
        </div>
        <div class="form-group">
            {{ form.confirm_password.label(class="form-label") }}
            {{ form.confirm_password(class="form-control") }}
            {% if form.confirm_password.errors %}
            <div class="invalid-feedback">
                {% for error in form.confirm_password.errors %}
                <span>{{ error }}</span>
                {% endfor %}
            </div>
            {% endif %}
        </div>
        <button type="submit" class="btn btn-primary">{{ form.submit.label }}</button>
    </form>
</div>
{% endblock %}

`base.html`

Ensure base.html remains the same, providing the base structure and handling flash messages.

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>{% block title %}{% endblock %}</title>
    <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/css/bootstrap.min.css">
</head>
<body>
    <nav class="navbar navbar-expand-lg navbar-light bg-light">
        <a class="navbar-brand" href="{{ url_for('home') }}">App</a>
        <div class="collapse navbar-collapse">
            <ul class="navbar-nav ml-auto">
                {% if current_user.is_authenticated %}
                <li class="nav-item">
                    <a class="nav-link" href="{{ url_for('logout') }}">Logout</a>
                </li>
                {% else %}
                <li class="nav-item">
                    <a class="nav-link" href="{{ url_for('login') }}">Login</a>
                </li>
                <li class="nav-item">
                    <a class="nav-link" href="{{ url_for('signup') }}">Sign Up</a>
                </li>
                {% endif %}
            </ul>
        </div>
    </nav>
    <div class="container">
        {% with messages = get_flashed_messages(with_categories=true) %}
        {% if messages %}
        {% for category, message in messages %}
        <div class="alert alert-{{ category }}">{{ message }}</div>
        {% endfor %}
        {% endif %}
        {% endwith %}
        {% block content %}{% endblock %}
    </div>
</body>
</html>

With this setup, the majority of the form handling and validation is managed by Flask-WTF, simplifying the front-end.

You won't really need AJAX here.

kylelhk commented 4 months ago

@iheathers If you wish, you may overwrite all the works I've done for the login and signup functions.

kylelhk commented 4 months ago

The validation process appears overly complex, suggesting possible over-engineering. It may be more efficient to handle all the logic on the server side using WTForms.

Are all functions still necessary if you want to retain them on the client side?

It was mentioned in one of the lectures that both client-side and serve-side validations should be in place.

trieuH commented 4 months ago

I'm glad you got the error messages displaying the way you wanted. All the functionality seems to be there, I think we can keep the code as is (just with removal of magic numbers)

iheathers commented 4 months ago

I am not requesting any change at the moment. Just adding here as a discussion of potential implementation. It's too late to make the change now.

kylelhk commented 4 months ago

@iheathers Could you please revert your commit for resolving conflicts as it has affected some changes previously made in this PR? That would be easier for me to track my changes and further work on other review comments. Thanks.

kylelhk / Pictionary