diff --git a/app/routes/auth.py b/app/routes/auth.py index ff0d383..0bff41d 100644 --- a/app/routes/auth.py +++ b/app/routes/auth.py @@ -2,12 +2,12 @@ This module handles user authentication including login with account lockout protection, logout with session clearing, and new user registration with -password policy enforcement and CAPTCHA verification. +password policy enforcement and sign-up screening. """ import os import secrets -import uuid +import time from datetime import datetime, timedelta from urllib.parse import urlencode, urlparse @@ -38,6 +38,18 @@ MAX_LOCKOUT_MINUTES = 240 #: response time stops telling a caller which usernames exist (SEC-017). _ABSENT_USER_HASH = None +#: Session key recording when the registration form was handed out. +REGISTRATION_ISSUED_KEY = 'registration_form_issued_at' + +#: Name of the honeypot input. Plausible enough that a form-filler wants it, +#: absent from the visible form. Hidden by .honeypot in style.css — not by an +#: inline style, so that the rule survives a tightening of style-src. +REGISTRATION_HONEYPOT_FIELD = 'website' + +#: Floor on how long a genuine registration takes. Eleven fields and a +#: password typed twice; three seconds is generous. +MIN_REGISTRATION_SECONDS = 3 + # Discord OAuth2 configuration DISCORD_CLIENT_ID = os.getenv('DISCORD_CLIENT_ID') DISCORD_CLIENT_SECRET = os.getenv('DISCORD_CLIENT_SECRET') @@ -116,41 +128,65 @@ def cooloff_minutes(failed_attempts): return min(LOCKOUT_DURATION_MINUTES * (2**steps), MAX_LOCKOUT_MINUTES) -def generate_captcha(): - """Generate a simple math CAPTCHA challenge. +def issue_registration_challenge(): + """Mark that the registration form has been handed out, and when. - Creates a random addition problem and stores the answer in the session. + Kept in the signed session rather than in a form field, so that the + timestamp is not something the submitter can choose. Left in place across + a failed submission: someone correcting a typo should not be told to slow + down, and a robot has already paid for the round trip by then. + """ + session.setdefault(REGISTRATION_ISSUED_KEY, time.time()) + + +def check_registration_challenge(form): + """Say why this registration should be refused, or None to accept. + + What replaced the arithmetic CAPTCHA, and why (SEC-AUTH-008). + + `a + b = ?` with both operands between 1 and 10 has nineteen possible + answers and is solvable by reading the string. It stopped no automated + registration whatsoever. What it did do was add a step for every human, + including anyone using a screen reader, in exchange for an appearance of + protection — which is worse than no protection, because it gets counted + as one. + + The audit's alternative was a real CAPTCHA service. That means a third + party, an API key, a request on every page load, and putting a foreign + script back into script-src — undoing the CSP work that closed + SEC-WEB-001. Disproportionate for a club site. + + So: two checks that cost the visitor nothing. + + - a honeypot field, hidden in the stylesheet, that a form-filling + robot completes and a person never sees; + - a minimum dwell time between being handed the form and sending it + back. Eleven fields and a password typed twice do not get filled in + under three seconds, and a POST with no issued form at all never + fetched the page. + + Be clear about the ceiling: this stops commodity spam, not somebody who + looks at the form for five minutes. The thing that would actually gate + registration is staff activation of new accounts, which does not exist — + `is_active_account` defaults to True. That is a product decision, not one + to slip in here. + + The session-forgery angle in the constat is moot: with SECRET_KEY + compromised (SEC-001) an attacker forges a logged-in session for any + account and has no reason to register at all. Returns: - dict: A dictionary with 'question' (e.g., '3 + 7') and 'id' keys. + str | None: a short reason for the log, or None to let it through. """ - import random + if form.get(REGISTRATION_HONEYPOT_FIELD, '').strip(): + return 'honeypot' - a = random.randint(1, 10) - b = random.randint(1, 10) - captcha_id = str(uuid.uuid4()) - session['captcha_id'] = captcha_id - session['captcha_answer'] = a + b - return {'question': f'{a} + {b} = ?', 'id': captcha_id} - - -def verify_captcha(user_answer): - """Verify the CAPTCHA answer from the session. - - Args: - user_answer: The user's submitted answer (string or int). - - Returns: - bool: True if the answer matches the stored CAPTCHA, False otherwise. - """ - try: - expected = session.pop('captcha_answer', None) - session.pop('captcha_id', None) - if expected is None: - return False - return int(user_answer) == expected - except (ValueError, TypeError): - return False + issued_at = session.get(REGISTRATION_ISSUED_KEY) + if issued_at is None: + return 'no-form-issued' + if time.time() - issued_at < MIN_REGISTRATION_SECONDS: + return 'too-fast' + return None auth_bp = Blueprint('auth', __name__, url_prefix='/auth') @@ -276,14 +312,33 @@ def login(): return render_template('pages/login.html') +def _rerender_registration(form_data): + """Re-render the registration form after a refusal. + + Was copied out four times, near-identically (ARCH-005). Dropping the two + password fields is the part that must not be forgotten in the fifth copy: + echoing a password back into the HTML puts it in the browser's cache and + in any proxy log along the way. + """ + form_data = dict(form_data) + form_data.pop('password', None) + form_data.pop('confirm_password', None) + return render_template( + 'pages/register.html', + esport_games=ESPORT_GAMES, + honeypot_field=REGISTRATION_HONEYPOT_FIELD, + form_data=form_data, + ) + + @auth_bp.route('/register', methods=['GET', 'POST']) @limiter.limit("20 per hour") def register(): - """Handle new player registration with CAPTCHA and password policy. + """Handle new player registration. - GET: Render the registration form with E-Sports games list and CAPTCHA. - POST: Validate all inputs, verify CAPTCHA, enforce password policy, - and create a new player account. + GET: Render the registration form with the E-Sports games list. + POST: Screen the submission (see check_registration_challenge), validate + every input against RegisterSchema, and create a new player account. Only players can register through this form. Validates username/email uniqueness and password confirmation. @@ -299,20 +354,15 @@ def register(): form_data = dict(request.form) form_data['games'] = request.form.getlist('games') - # Validate CAPTCHA first - captcha_answer = request.form.get('captcha_answer', '') - if not verify_captcha(captcha_answer): - flash(_('Incorrect CAPTCHA answer. Please try again.'), 'danger') - captcha = generate_captcha() - # Clear password fields only on CAPTCHA failure - form_data.pop('password', None) - form_data.pop('confirm_password', None) - return render_template( - 'pages/register.html', - esport_games=ESPORT_GAMES, - captcha=captcha, - form_data=form_data, - ) + refusal = check_registration_challenge(request.form) + if refusal is not None: + # Logged, because this is the only place abuse of the sign-up + # form becomes visible at all. Deliberately vague to the sender: + # naming the honeypot tells whoever tripped it how to avoid it. + log_auth_event('account.registration_refused', reason=refusal) + flash(_('Your registration could not be processed. Please try again.'), 'danger') + issue_registration_challenge() + return _rerender_registration(form_data) # Validate input with marshmallow schema register_schema = RegisterSchema() @@ -322,16 +372,7 @@ def register(): for field, messages in err.messages.items(): for msg in messages: flash(_('%(field)s: %(msg)s', field=field, msg=msg), 'danger') - captcha = generate_captcha() - # Clear password fields on validation failure - form_data.pop('password', None) - form_data.pop('confirm_password', None) - return render_template( - 'pages/register.html', - esport_games=ESPORT_GAMES, - captcha=captcha, - form_data=form_data, - ) + return _rerender_registration(form_data) username = validated['username'] email = validated['email'] @@ -345,27 +386,11 @@ def register(): if User.query.filter_by(username=username).first(): flash(_('Username already exists.'), 'danger') - captcha = generate_captcha() - form_data.pop('password', None) - form_data.pop('confirm_password', None) - return render_template( - 'pages/register.html', - esport_games=ESPORT_GAMES, - captcha=captcha, - form_data=form_data, - ) + return _rerender_registration(form_data) if User.query.filter_by(email=email).first(): flash(_('Email already registered.'), 'danger') - captcha = generate_captcha() - form_data.pop('password', None) - form_data.pop('confirm_password', None) - return render_template( - 'pages/register.html', - esport_games=ESPORT_GAMES, - captcha=captcha, - form_data=form_data, - ) + return _rerender_registration(form_data) hashed_password = hash_password(password) user = Player( @@ -404,6 +429,7 @@ def register(): # Clear Discord OAuth data from session after successful registration session.pop('discord_oauth', None) + session.pop(REGISTRATION_ISSUED_KEY, None) log_auth_event('account.registered', username=user.username, user_id=user.id) @@ -411,13 +437,8 @@ def register(): return redirect(url_for('auth.login')) # GET request — render empty form - captcha = generate_captcha() - return render_template( - 'pages/register.html', - esport_games=ESPORT_GAMES, - captcha=captcha, - form_data={}, - ) + issue_registration_challenge() + return _rerender_registration({}) @auth_bp.route('/discord/login') diff --git a/app/static/css/style.css b/app/static/css/style.css index 48cff1e..20d2aea 100644 --- a/app/static/css/style.css +++ b/app/static/css/style.css @@ -2021,3 +2021,12 @@ a:hover { color: var(--primary-dark); } white-space: nowrap; border: 0; } + +/* Honeypot: hidden from everyone, filled in only by a robot (SEC-AUTH-008). + The opposite of .sr-only above — that one hides from the eye and keeps the + announcement, this one has to hide from both. display:none is deliberate: + an off-screen input is still reachable by keyboard and by a screen reader, + and a person who lands in it gets refused with no idea why. */ +.honeypot { + display: none; +} diff --git a/app/templates/pages/register.html b/app/templates/pages/register.html index dc8a03b..9534206 100644 --- a/app/templates/pages/register.html +++ b/app/templates/pages/register.html @@ -146,9 +146,14 @@ required> -