Sign in Get started

Code review · Python

Review a Sign-Up Function

Medium Code review

This function backs the sign-up form of a web app. It runs in several web workers behind a load balancer.

Review it: list every problem, why it matters, and how you would fix it.

The code to review

There are 5 issues to find.

import hashlib

from app.db import db


class SignupError(Exception):
    pass


def register(email: str, password: str) -> int:
    existing = db.query_one("SELECT id FROM users WHERE email = %s", (email,))
    if existing:
        raise SignupError(f"{email} is already registered")

    password_hash = hashlib.md5(password.encode()).hexdigest()
    user_id = db.insert(
        "INSERT INTO users (email, password_hash) VALUES (%s, %s) RETURNING id",
        (email, password_hash),
    )
    return user_id

Write your review

Loading the editor… (JavaScript is required to run code in your browser.)

The issues and how to fix them

Show the issues

1. Fast, unsalted password hash (MD5)

Critical, line 15

MD5 is designed to be fast: a GPU can try billions of guesses per second, and without a salt identical passwords have identical hashes, so leaked hashes are cracked with precomputed tables.

Fix: Use a slow, salted password hashing function: Argon2id (for example argon2-cffi) or bcrypt, with parameters tuned to roughly 100 ms.

2. Check-then-insert race

Major, line 11-19

Two requests with the same email can both pass the SELECT before either inserts, creating duplicate accounts. Load balancing across workers makes this likely during double-clicks and retries.

Fix: Enforce uniqueness in the database (UNIQUE index on the normalised email) and handle the unique-violation error from the insert, instead of checking first.

3. Email is not normalised

Major, line 11

[email protected] and [email protected] are treated as different users, which breaks login, password reset and the duplicate check.

Fix: Normalise (trim and lowercase) before storing and comparing, or use a case-insensitive column type such as PostgreSQL citext.

4. Error message enables account enumeration

Minor, line 13

"x is already registered" lets an attacker test which emails have accounts, useful for phishing and credential stuffing.

Fix: Return the same response either way and send an email to the address ("someone tried to sign up with your address").

5. No password policy

Minor, line 10

Empty or one-character passwords are accepted.

Fix: Check a minimum length (NIST suggests at least 8, more for admin accounts) and reject known-breached passwords; avoid arbitrary composition rules.

The password hash is the critical finding; the race condition is the one that separates strong candidates, because it only appears under concurrency. The general rule: let the database enforce invariants such as uniqueness, then handle the error.

Fixed version

from argon2 import PasswordHasher

from app.db import UniqueViolation, db

hasher = PasswordHasher()  # Argon2id with a random salt per hash


class SignupError(Exception):
    pass


def register(email: str, password: str) -> int | None:
    email = email.strip().lower()
    if len(password) < 10:
        raise SignupError("Password must be at least 10 characters")
    try:
        return db.insert(
            "INSERT INTO users (email, password_hash) VALUES (%s, %s) RETURNING id",
            (email, hasher.hash(password)),
        )
    except UniqueViolation:  # UNIQUE index on users(email) makes this race-free
        notify_existing_account(email)
        return None  # caller shows the same "check your email" message either way

Follow-up questions

  • How would you migrate existing MD5 hashes without forcing every user to reset?
  • Where else does check-then-act appear? (inventory, coupons, rate limits)