Merge branch 'dev' of https://git.immortal.host/clubesportsudes/team-tryouts into dev
This commit is contained in:
+457
-110
@@ -5,18 +5,64 @@ the Flask application instance with comprehensive security hardening.
|
||||
"""
|
||||
|
||||
import os
|
||||
from flask import Flask, request, redirect, jsonify, render_template, url_for
|
||||
from flask_cors import CORS
|
||||
from app.extensions import db, login_manager, csrf, hash_password, check_password, limiter
|
||||
from sqlalchemy import text
|
||||
from werkzeug.exceptions import HTTPException
|
||||
import secrets
|
||||
|
||||
import markupsafe
|
||||
from dotenv import load_dotenv
|
||||
import os as _os
|
||||
from flask import (
|
||||
Flask,
|
||||
current_app,
|
||||
flash,
|
||||
g,
|
||||
jsonify,
|
||||
redirect,
|
||||
render_template,
|
||||
request,
|
||||
url_for,
|
||||
)
|
||||
from flask_babel import gettext as _
|
||||
from flask_cors import CORS
|
||||
from sqlalchemy import text
|
||||
from werkzeug.exceptions import HTTPException
|
||||
|
||||
from app import i18n
|
||||
from app.extensions import babel, csrf, db, limiter, login_manager
|
||||
from app.pagination import page_url
|
||||
|
||||
load_dotenv()
|
||||
|
||||
#: Fallback for requests that never reached a view: a 404 on an unrouted
|
||||
#: path has no endpoint to read a mark off, and `/users/api/typo` should
|
||||
#: still answer a fetch() in JSON.
|
||||
#:
|
||||
#: Not the primary mechanism. Seven error handlers each carried their own
|
||||
#: copy of a list like this (STD-09); the copies had drifted, and all seven
|
||||
#: were missing the same endpoints. Views now mark themselves — see
|
||||
#: `app/api.py` for why a prefix list could not have been made correct.
|
||||
JSON_URL_PREFIXES = (
|
||||
'/users/disponibilities',
|
||||
'/users/coach-availability',
|
||||
'/users/api/',
|
||||
'/matches/api/',
|
||||
'/team-matches/api/',
|
||||
)
|
||||
|
||||
|
||||
def wants_json_response():
|
||||
"""Whether this request must be answered with JSON rather than an HTML page.
|
||||
|
||||
Three signals, in order of authority: the view said so (`@json_endpoint`),
|
||||
the path is under a JSON prefix (for requests that matched no view at
|
||||
all), or the caller asked for JSON and nothing else.
|
||||
"""
|
||||
view = current_app.view_functions.get(request.endpoint) if request.endpoint else None
|
||||
if getattr(view, 'returns_json', False):
|
||||
return True
|
||||
if request.path.startswith(JSON_URL_PREFIXES):
|
||||
return True
|
||||
accept = request.accept_mimetypes
|
||||
return accept.best == 'application/json' and not accept.accept_html
|
||||
|
||||
# Load .env from the app directory (independent of the process working directory)
|
||||
_env_path = _os.path.join(_os.path.dirname(_os.path.abspath(__file__)), '.env')
|
||||
load_dotenv(_env_path)
|
||||
|
||||
def nl2br(value):
|
||||
"""Convert newlines to HTML line breaks.
|
||||
@@ -28,13 +74,103 @@ def nl2br(value):
|
||||
Markup: HTML-safe string with line breaks.
|
||||
"""
|
||||
if value:
|
||||
return markupsafe.Markup('<br>'.join(str(value).splitlines()))
|
||||
# Markup('<br>').join() escapes each segment before joining.
|
||||
# Markup('<br>'.join(...)) would mark attacker-controlled text as safe.
|
||||
return markupsafe.Markup('<br>').join(str(value).splitlines())
|
||||
return ''
|
||||
|
||||
|
||||
def create_app():
|
||||
def normalise_database_url(url):
|
||||
"""Name the PostgreSQL driver explicitly in a connection URL.
|
||||
|
||||
`postgresql://…` does not mean "whichever driver is installed": it means
|
||||
psycopg2, which SQLAlchemy imports at create_engine() time. requirements
|
||||
.txt pins psycopg 3 (`psycopg[binary]`) and no psycopg2, so a clean
|
||||
install starting against the URL Render hands out — and the one this
|
||||
project's own documentation shows — raises
|
||||
|
||||
ModuleNotFoundError: No module named 'psycopg2'
|
||||
|
||||
before the first request. Anything with a driver already spelled out
|
||||
(`postgresql+psycopg://`, `postgresql+psycopg2://`) is left alone, so
|
||||
naming psycopg2 stays possible for an environment that has it.
|
||||
|
||||
`postgres://` is the legacy alias several hosts still emit; SQLAlchemy
|
||||
dropped it in 1.4.
|
||||
|
||||
Args:
|
||||
url: Value of DATABASE_URL, or None.
|
||||
|
||||
Returns:
|
||||
str | None: The URL, with a driver named when it was PostgreSQL.
|
||||
"""
|
||||
if not url:
|
||||
return url
|
||||
scheme, separator, rest = url.partition('://')
|
||||
if not separator or '+' in scheme:
|
||||
return url
|
||||
if scheme in ('postgres', 'postgresql'):
|
||||
return f'postgresql+psycopg://{rest}'
|
||||
return url
|
||||
|
||||
|
||||
def build_csp(*, allow_inline_script, nonce=None):
|
||||
"""Assemble the Content-Security-Policy header.
|
||||
|
||||
Two mutually exclusive modes, and they really are exclusive.
|
||||
|
||||
Under CSP level 3, a browser that understands nonces **ignores
|
||||
'unsafe-inline' entirely as soon as a nonce is present**. Emitting both
|
||||
would therefore not be a gentle transition: it would drop every inline
|
||||
script and every onclick attribute at once, in modern browsers only.
|
||||
The switch has to be atomic, which is why one flag drives it.
|
||||
|
||||
While allow_inline_script is true no nonce is emitted at all, so adding
|
||||
nonce="{{ csp_nonce }}" to a template ahead of the switch is harmless.
|
||||
|
||||
Flipping the flag requires every inline event handler to be gone first.
|
||||
A nonce cannot authorise an onclick attribute — nonces apply to script
|
||||
elements, never to handler attributes. See tests/test_csp.py, which
|
||||
tracks how many are left.
|
||||
|
||||
Args:
|
||||
allow_inline_script: Keep 'unsafe-inline' in script-src.
|
||||
nonce: Per-request nonce, used only when inline script is not allowed.
|
||||
|
||||
Returns:
|
||||
str: The header value.
|
||||
"""
|
||||
if allow_inline_script:
|
||||
script_src = "'self' 'unsafe-inline' https://cdn.jsdelivr.net"
|
||||
else:
|
||||
script_src = f"'self' 'nonce-{nonce}' https://cdn.jsdelivr.net"
|
||||
|
||||
return '; '.join(
|
||||
[
|
||||
"default-src 'self'",
|
||||
f'script-src {script_src}',
|
||||
# style-src is a separate migration: inline style="" attributes are
|
||||
# spread across the templates and are not an XSS vector on their own.
|
||||
"style-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com https://cdn.jsdelivr.net",
|
||||
"font-src 'self' https://cdnjs.cloudflare.com",
|
||||
"img-src 'self' data: https://cdn.discordapp.com",
|
||||
"connect-src 'self'",
|
||||
"frame-ancestors 'none'",
|
||||
"base-uri 'self'",
|
||||
"form-action 'self'",
|
||||
]
|
||||
)
|
||||
|
||||
|
||||
def create_app(config=None):
|
||||
"""Create and configure the Flask application.
|
||||
|
||||
Args:
|
||||
config: Optional mapping of configuration overrides, applied after the
|
||||
environment defaults and before validation. This is what makes the
|
||||
factory usable from tests: pass a throwaway database URI, a dummy
|
||||
secret, and turn off the Discord bot, without touching os.environ.
|
||||
|
||||
Initializes Flask with:
|
||||
- Secret key for session security
|
||||
- Database configuration
|
||||
@@ -54,36 +190,103 @@ def create_app():
|
||||
Flask: Configured Flask application instance.
|
||||
"""
|
||||
app = Flask(__name__)
|
||||
|
||||
# Cache-busting stamps for static files, filled lazily by the url_defaults
|
||||
# hook below. Per application instance, so the test suite does not carry
|
||||
# one app's mtimes into the next.
|
||||
_static_stamps = {}
|
||||
|
||||
# --- defaults from the environment ------------------------------------
|
||||
app.config['SECRET_KEY'] = os.getenv('SECRET_KEY')
|
||||
app.config['SQLALCHEMY_DATABASE_URI'] = os.getenv('DATABASE_URL')
|
||||
app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
|
||||
app.config['WTF_CSRF_ENABLED'] = True
|
||||
app.config['CORS_ALLOWED_ORIGINS'] = os.getenv('CORS_ALLOWED_ORIGINS', '')
|
||||
app.config['FORCE_HTTPS'] = os.getenv('FORCE_HTTPS', 'true').lower() == 'true'
|
||||
|
||||
# Now false: every inline event handler has been replaced by a
|
||||
# data-action attribute dispatched from main.js, so script-src no longer
|
||||
# needs 'unsafe-inline'. Inline <script> blocks carry a per-request
|
||||
# nonce. The escape hatch remains for a deployment that hits an
|
||||
# overlooked handler — but leaving it on gives up the protection that
|
||||
# would have blocked SEC-XSS-001.
|
||||
app.config['CSP_ALLOW_INLINE_SCRIPT'] = (
|
||||
os.getenv('CSP_ALLOW_INLINE_SCRIPT', 'false').lower() == 'true'
|
||||
)
|
||||
|
||||
# Internationalisation. French is the site's primary language.
|
||||
app.config['BABEL_DEFAULT_LOCALE'] = i18n.DEFAULT_LOCALE
|
||||
app.config['BABEL_TRANSLATION_DIRECTORIES'] = os.path.join(
|
||||
os.path.dirname(os.path.abspath(__file__)), 'translations'
|
||||
)
|
||||
|
||||
# Side effects of create_app(), both on by default so that production and
|
||||
# development behave exactly as before. Tests turn them off.
|
||||
app.config['AUTO_CREATE_TABLES'] = os.getenv('AUTO_CREATE_TABLES', 'true').lower() == 'true'
|
||||
app.config['ENABLE_DISCORD_BOT'] = os.getenv('ENABLE_DISCORD_BOT', 'true').lower() == 'true'
|
||||
|
||||
# Where the rate limiter keeps its counters (SEC-WEB-004).
|
||||
#
|
||||
# `memory://` is what Flask-Limiter falls back to when nothing is set,
|
||||
# and it is the correct choice here: Waitress serves this application
|
||||
# from one process, so one set of counters in that process is all there
|
||||
# is to share. Naming it changes nothing at runtime and two things
|
||||
# otherwise — it stops being an accident, and it becomes settable to a
|
||||
# Redis URI on the day the deployment gains a second process, which is
|
||||
# the day in-memory counters would start letting through N times the
|
||||
# configured limit without anyone noticing.
|
||||
#
|
||||
# What this does not fix: the counters are keyed on an IP address that
|
||||
# is forgeable while TRUSTED_PROXY is unresolved (OPS-002). Shared
|
||||
# storage for a forgeable key buys nothing, which is why that one is
|
||||
# the prerequisite and not this.
|
||||
app.config['RATELIMIT_STORAGE_URI'] = os.getenv('RATELIMIT_STORAGE_URI', 'memory://')
|
||||
|
||||
# --- caller overrides win ---------------------------------------------
|
||||
if config:
|
||||
app.config.update(config)
|
||||
|
||||
# --- validation, after overrides so tests can supply their own ---------
|
||||
if not app.config['SECRET_KEY']:
|
||||
raise RuntimeError('SECRET_KEY environment variable must be set for security')
|
||||
database_url = os.getenv('DATABASE_URL')
|
||||
if not database_url:
|
||||
if not app.config['SQLALCHEMY_DATABASE_URI']:
|
||||
raise RuntimeError(
|
||||
'DATABASE_URL environment variable must be set to a PostgreSQL connection string'
|
||||
)
|
||||
# Use psycopg2 driver for PostgreSQL
|
||||
if database_url.startswith('postgresql://'):
|
||||
database_url = database_url.replace('postgresql://', 'postgresql+psycopg2://', 1)
|
||||
elif database_url.startswith('postgres://'):
|
||||
database_url = database_url.replace('postgres://', 'postgresql+psycopg2://', 1)
|
||||
app.config['SQLALCHEMY_DATABASE_URI'] = database_url
|
||||
app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
|
||||
app.config['WTF_CSRF_ENABLED'] = True
|
||||
|
||||
# After the overrides, so a caller-supplied URL is normalised too.
|
||||
app.config['SQLALCHEMY_DATABASE_URI'] = normalise_database_url(
|
||||
app.config['SQLALCHEMY_DATABASE_URI']
|
||||
)
|
||||
|
||||
# File upload size limit (16 MB)
|
||||
app.config['MAX_CONTENT_LENGTH'] = 16 * 1024 * 1024
|
||||
|
||||
# Secure session cookie settings
|
||||
app.config['SESSION_COOKIE_SECURE'] = os.getenv('SESSION_COOKIE_SECURE', 'true').lower() == 'true'
|
||||
app.config['SESSION_COOKIE_SECURE'] = (
|
||||
os.getenv('SESSION_COOKIE_SECURE', 'true').lower() == 'true'
|
||||
)
|
||||
app.config['SESSION_COOKIE_HTTPONLY'] = True
|
||||
app.config['SESSION_COOKIE_SAMESITE'] = 'Lax'
|
||||
app.config['PERMANENT_SESSION_LIFETIME'] = 3600 # 1 hour session timeout
|
||||
|
||||
# Configure CORS - restrict to specific origins in production
|
||||
allowed_origins = os.getenv('CORS_ALLOWED_ORIGINS', '').split(',')
|
||||
allowed_origins = str(app.config.get('CORS_ALLOWED_ORIGINS') or '').split(',')
|
||||
allowed_origins = [origin.strip() for origin in allowed_origins if origin.strip()]
|
||||
|
||||
|
||||
# CORS is only configured when origins are named explicitly.
|
||||
#
|
||||
# The previous else-branch called CORS(app, supports_credentials=True)
|
||||
# with no origins argument. flask-cors then defaults to '*' and, because
|
||||
# credentials are allowed, echoes back whatever Origin the caller sent
|
||||
# together with Access-Control-Allow-Credentials: true — the opposite of
|
||||
# the "allow all (development) or none (production)" the comment claimed.
|
||||
#
|
||||
# Exploitation was blocked by SESSION_COOKIE_SAMESITE = 'Lax', which stops
|
||||
# the browser attaching the session cookie to a cross-site fetch. That is
|
||||
# a single setting standing between a misconfiguration and a cross-origin
|
||||
# data leak. This application renders server-side HTML on one origin and
|
||||
# needs no CORS policy at all.
|
||||
if allowed_origins:
|
||||
CORS(
|
||||
app,
|
||||
@@ -92,33 +295,122 @@ def create_app():
|
||||
methods=['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'],
|
||||
max_age=3600, # Cache preflight for 1 hour
|
||||
)
|
||||
else:
|
||||
# When no origins specified, allow all (development) or none (production)
|
||||
# In production with a reverse proxy, CORS is handled at the Nginx level
|
||||
CORS(
|
||||
app,
|
||||
supports_credentials=True,
|
||||
methods=['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'],
|
||||
max_age=3600,
|
||||
)
|
||||
|
||||
db.init_app(app)
|
||||
login_manager.init_app(app)
|
||||
csrf.init_app(app)
|
||||
limiter.init_app(app)
|
||||
|
||||
# Said once, at startup, because the failure mode is silent: counters in
|
||||
# process memory are lost on every restart and are not shared, so a
|
||||
# second worker would double every limit and nothing would report it.
|
||||
if app.config['RATELIMIT_STORAGE_URI'].startswith('memory://'):
|
||||
app.logger.info(
|
||||
'Rate limiting counters are held in process memory. Correct for a '
|
||||
'single-process deployment; set RATELIMIT_STORAGE_URI to a shared '
|
||||
'backend before running more than one worker (SEC-WEB-004).'
|
||||
)
|
||||
else:
|
||||
app.logger.info(
|
||||
'Rate limiting counters are held in a shared backend (%s).',
|
||||
app.config['RATELIMIT_STORAGE_URI'].split('://', 1)[0],
|
||||
)
|
||||
|
||||
babel.init_app(app, locale_selector=i18n.select_locale)
|
||||
|
||||
# Exposed to every template so the language switcher can render itself
|
||||
# without each view having to pass the list along.
|
||||
@app.before_request
|
||||
def generate_csp_nonce():
|
||||
# Only meaningful once inline script is disallowed; generated
|
||||
# unconditionally so templates can carry nonce="" beforehand.
|
||||
g.csp_nonce = secrets.token_urlsafe(16)
|
||||
|
||||
@app.before_request
|
||||
def assign_request_id():
|
||||
"""Give this request a name, so its log lines can be found (OBS-005).
|
||||
|
||||
Every record emitted while handling it carries this id — see
|
||||
RequestIdFilter — which is what turns "an error happened around
|
||||
14:32" into the six lines that led to it. It goes back in
|
||||
X-Request-Id and onto the 500 page, so that a report of "it broke
|
||||
when I clicked save" is enough to find the trace.
|
||||
|
||||
Generated here, never taken from an inbound header: with no trusted
|
||||
proxy settled (OPS-002), an accepted header lets any caller write
|
||||
arbitrary text — newlines included — into the log file.
|
||||
"""
|
||||
g.request_id = secrets.token_hex(8)
|
||||
|
||||
@app.after_request
|
||||
def expose_request_id(response):
|
||||
response.headers['X-Request-Id'] = g.get('request_id', '-')
|
||||
return response
|
||||
|
||||
@app.url_defaults
|
||||
def version_static_urls(endpoint, values):
|
||||
"""Stamp every static URL with the file's modification time.
|
||||
|
||||
Without this, nginx cannot be allowed to cache style.css and main.js:
|
||||
their URLs never change, so a 30-day expiry means a 30-day-old stylesheet
|
||||
with no way to invalidate it short of telling people to hard-refresh.
|
||||
With it, a deployed file gets a new URL and the old entry simply stops
|
||||
being asked for — which is what makes the `immutable` in nginx.conf
|
||||
true rather than merely fast (PERF-006).
|
||||
|
||||
The stamp is computed once per file per process. The process restarts
|
||||
on deploy, which is exactly when a file can have changed.
|
||||
"""
|
||||
if endpoint != 'static' or 'filename' not in values:
|
||||
return
|
||||
filename = values['filename']
|
||||
stamp = _static_stamps.get(filename)
|
||||
if stamp is None:
|
||||
try:
|
||||
stamp = str(int(os.stat(os.path.join(app.static_folder, filename)).st_mtime))
|
||||
except OSError:
|
||||
# A missing file is the template's problem, not this hook's:
|
||||
# let the URL build and let the 404 say so.
|
||||
stamp = ''
|
||||
_static_stamps[filename] = stamp
|
||||
if stamp:
|
||||
values['v'] = stamp
|
||||
|
||||
@app.context_processor
|
||||
def inject_csp_nonce():
|
||||
return {
|
||||
'csp_nonce': '' if app.config['CSP_ALLOW_INLINE_SCRIPT'] else g.get('csp_nonce', '')
|
||||
}
|
||||
|
||||
@app.context_processor
|
||||
def inject_locales():
|
||||
from flask_babel import get_locale
|
||||
|
||||
return {
|
||||
'current_locale': str(get_locale() or i18n.DEFAULT_LOCALE),
|
||||
'supported_locales': i18n.SUPPORTED_LOCALES,
|
||||
'locale_names': i18n.LOCALE_NAMES,
|
||||
}
|
||||
|
||||
# Used by layouts/_pagination.html. A global rather than something each
|
||||
# listing passes, because the thing that goes wrong with pagination links
|
||||
# is dropping the rest of the query string — `sort`, `order`, `team_id` —
|
||||
# and that is easier to get right once than in four templates (MNT-14).
|
||||
app.jinja_env.globals['page_url'] = page_url
|
||||
|
||||
# Configure structured logging
|
||||
from app.logging_config import configure_logging
|
||||
|
||||
configure_logging(app)
|
||||
|
||||
from app.routes.auth import auth_bp
|
||||
from app.routes.tryouts import tryouts_bp
|
||||
from app.routes.evaluations import evaluations_bp
|
||||
from app.routes.users import users_bp
|
||||
from app.routes.main import main_bp
|
||||
from app.routes.teams import teams_bp
|
||||
from app.routes.matches import matches_bp
|
||||
from app.routes.team_matches import team_matches_bp
|
||||
from app.routes.teams import teams_bp
|
||||
from app.routes.tryouts import tryouts_bp
|
||||
from app.routes.users import users_bp
|
||||
from app.routes.admin import admin_bp
|
||||
|
||||
app.register_blueprint(auth_bp)
|
||||
@@ -147,25 +439,20 @@ def create_app():
|
||||
HSTS is only sent in production (non-debug) to avoid breaking
|
||||
local development over plain HTTP.
|
||||
"""
|
||||
# X-XSS-Protection is deliberately not set: the auditor it addressed
|
||||
# has been removed from every current browser, and its last versions
|
||||
# introduced vulnerabilities of their own. CSP frame-ancestors and
|
||||
# X-Frame-Options cover the remaining ground.
|
||||
response.headers['X-Content-Type-Options'] = 'nosniff'
|
||||
response.headers['X-Frame-Options'] = 'DENY'
|
||||
response.headers['X-XSS-Protection'] = '1; mode=block'
|
||||
response.headers['Referrer-Policy'] = 'strict-origin-when-cross-origin'
|
||||
response.headers['Permissions-Policy'] = (
|
||||
'camera=(), microphone=(), geolocation=(), '
|
||||
'interest-cohort=(), payment=(), usb=()'
|
||||
'camera=(), microphone=(), geolocation=(), interest-cohort=(), payment=(), usb=()'
|
||||
)
|
||||
response.headers['Cross-Origin-Opener-Policy'] = 'same-origin'
|
||||
response.headers['Content-Security-Policy'] = (
|
||||
"default-src 'self'; "
|
||||
"script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; "
|
||||
"style-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com https://cdn.jsdelivr.net; "
|
||||
"font-src 'self' https://cdnjs.cloudflare.com; "
|
||||
"img-src 'self' data: https://cdn.discordapp.com; "
|
||||
"connect-src 'self'; "
|
||||
"frame-ancestors 'none'; "
|
||||
"base-uri 'self'; "
|
||||
"form-action 'self'"
|
||||
response.headers['Content-Security-Policy'] = build_csp(
|
||||
allow_inline_script=app.config['CSP_ALLOW_INLINE_SCRIPT'],
|
||||
nonce=g.get('csp_nonce'),
|
||||
)
|
||||
|
||||
# Only enable HSTS when HTTPS is actually being used
|
||||
@@ -187,11 +474,17 @@ def create_app():
|
||||
|
||||
Respects the X-Forwarded-Proto header from reverse proxies.
|
||||
Can be disabled via FORCE_HTTPS environment variable.
|
||||
|
||||
Returns:
|
||||
Response | None: A redirect, or None to let the request through.
|
||||
"""
|
||||
if not app.debug:
|
||||
if not request.is_secure and request.headers.get('X-Forwarded-Proto') != 'https':
|
||||
if os.getenv('FORCE_HTTPS', 'true').lower() == 'true':
|
||||
return redirect(request.url.replace('http://', 'https://'), code=301)
|
||||
if not app.debug and app.config['FORCE_HTTPS']:
|
||||
already_secure = (
|
||||
request.is_secure or request.headers.get('X-Forwarded-Proto') == 'https'
|
||||
)
|
||||
if not already_secure:
|
||||
return redirect(request.url.replace('http://', 'https://'), code=301)
|
||||
return None
|
||||
|
||||
# =========================================================================
|
||||
# Health Check Endpoint
|
||||
@@ -212,13 +505,28 @@ def create_app():
|
||||
'version': '1.0.0',
|
||||
}
|
||||
|
||||
# The bot runs in a daemon thread inside this process. When it dies
|
||||
# the site keeps serving pages and every notification stops, with
|
||||
# nothing to see from outside — which is how it stayed unnoticed.
|
||||
# Reported, not fatal: a club without Discord reminders is degraded,
|
||||
# not down, and a 503 here would take the site out of the load
|
||||
# balancer for it (OPS-012).
|
||||
if app.config['ENABLE_DISCORD_BOT']:
|
||||
from app.discord_bot import bot_status
|
||||
|
||||
health_data['discord_bot'] = bot_status()
|
||||
|
||||
# Check database connectivity
|
||||
try:
|
||||
db.session.execute(text('SELECT 1'))
|
||||
health_data['database'] = 'connected'
|
||||
except Exception as e:
|
||||
except Exception:
|
||||
# Never echo the driver error: it routinely carries the host,
|
||||
# database name and user of the connection string, and /health
|
||||
# is unauthenticated.
|
||||
app.logger.error('Health check: database unreachable', exc_info=True)
|
||||
health_data['status'] = 'unhealthy'
|
||||
health_data['database'] = f'error: {str(e)}'
|
||||
health_data['database'] = 'error'
|
||||
return jsonify(health_data), 503
|
||||
|
||||
return jsonify(health_data), 200
|
||||
@@ -226,6 +534,36 @@ def create_app():
|
||||
# =========================================================================
|
||||
# Custom Error Handlers
|
||||
# =========================================================================
|
||||
#
|
||||
# STD-09. The seven handlers below each carried their own copy of a list
|
||||
# of URL prefixes, and the copies had drifted: three of them checked
|
||||
# `/users/coach-availability`, four did not. Worse, every copy was
|
||||
# missing the same five endpoints — `/matches/api/…` and
|
||||
# `/team-matches/api/…` — so an error on any of those answered a
|
||||
# `fetch()` with an HTML error page. The browser then failed to parse it
|
||||
# as JSON and the page simply did nothing: on the calendar, the tryout
|
||||
# and team selects stayed empty with no message anywhere. A session that
|
||||
# expired mid-page produced exactly that, because the 401 handler
|
||||
# redirects to an HTML login form.
|
||||
#
|
||||
# The mechanism now lives in wants_json_response() / app/api.py, and a
|
||||
# test walks the URL map to prove no jsonify-returning view is missed.
|
||||
#
|
||||
# The handler below is the one that actually mattered. `@login_required`
|
||||
# never reaches the 401 handler: Flask-Login intercepts first and calls
|
||||
# its own unauthorized callback, which redirects. So every one of the
|
||||
# sixteen JSON endpoints answered an expired session with a 302 to an
|
||||
# HTML login form, whatever the prefix list said — and the page's
|
||||
# `fetch()` threw parsing it. Rewriting the prefix list alone would have
|
||||
# left this untouched and looked like a fix.
|
||||
@login_manager.unauthorized_handler
|
||||
def handle_unauthorized():
|
||||
"""What an unauthenticated request gets: a redirect, or a 401 in JSON."""
|
||||
if wants_json_response():
|
||||
return jsonify({'error': 'Unauthorized', 'message': 'Your session has expired.'}), 401
|
||||
flash(_('Please log in to access this page.'), 'warning')
|
||||
return redirect(url_for('auth.login'))
|
||||
|
||||
@app.errorhandler(400)
|
||||
def bad_request(error):
|
||||
"""Handle 400 Bad Request errors.
|
||||
@@ -236,28 +574,21 @@ def create_app():
|
||||
Returns:
|
||||
Response: Rendered error page or JSON for API requests.
|
||||
"""
|
||||
if request.path.startswith('/users/disponibilities') or \
|
||||
request.path.startswith('/users/coach-availability') or \
|
||||
request.path.startswith('/users/api/'):
|
||||
if wants_json_response():
|
||||
return jsonify({'error': 'Bad request', 'message': str(error)}), 400
|
||||
return render_template('errors/400.html', error=error), 400
|
||||
|
||||
@app.errorhandler(401)
|
||||
def unauthorized(error):
|
||||
"""Handle 401 Unauthorized errors.
|
||||
"""Handle an explicit abort(401).
|
||||
|
||||
Args:
|
||||
error: The error object.
|
||||
|
||||
Returns:
|
||||
Response: Redirect to login for pages, JSON for API.
|
||||
Rarely reached: `@login_required` is intercepted by Flask-Login
|
||||
before Flask's error handling, and answered by handle_unauthorized
|
||||
above. This covers code that aborts with 401 itself, and gives the
|
||||
same answer — the two used to differ, and the flash message here was
|
||||
the one string in the application that had never been translated.
|
||||
"""
|
||||
if request.path.startswith('/users/disponibilities') or \
|
||||
request.path.startswith('/users/api/'):
|
||||
return jsonify({'error': 'Unauthorized'}), 401
|
||||
from flask import flash as _flash
|
||||
_flash('Please log in to access this page.', 'warning')
|
||||
return redirect(url_for('auth.login'))
|
||||
return handle_unauthorized()
|
||||
|
||||
@app.errorhandler(403)
|
||||
def forbidden(error):
|
||||
@@ -269,8 +600,7 @@ def create_app():
|
||||
Returns:
|
||||
Response: Rendered error page or JSON for API requests.
|
||||
"""
|
||||
if request.path.startswith('/users/disponibilities') or \
|
||||
request.path.startswith('/users/api/'):
|
||||
if wants_json_response():
|
||||
return jsonify({'error': 'Forbidden', 'message': str(error)}), 403
|
||||
return render_template('errors/403.html', error=error), 403
|
||||
|
||||
@@ -284,8 +614,7 @@ def create_app():
|
||||
Returns:
|
||||
Response: Rendered error page or JSON for API requests.
|
||||
"""
|
||||
if request.path.startswith('/users/disponibilities') or \
|
||||
request.path.startswith('/users/api/'):
|
||||
if wants_json_response():
|
||||
return jsonify({'error': 'Not found'}), 404
|
||||
return render_template('errors/404.html', error=error), 404
|
||||
|
||||
@@ -299,12 +628,10 @@ def create_app():
|
||||
Returns:
|
||||
Response: JSON error for API or rendered page.
|
||||
"""
|
||||
if request.path.startswith('/users/disponibilities') or \
|
||||
request.path.startswith('/users/api/'):
|
||||
return jsonify({
|
||||
'error': 'Too many requests',
|
||||
'message': 'Please try again later.'
|
||||
}), 429
|
||||
if wants_json_response():
|
||||
return jsonify(
|
||||
{'error': 'Too many requests', 'message': 'Please try again later.'}
|
||||
), 429
|
||||
return render_template('errors/429.html', error=error), 429
|
||||
|
||||
@app.errorhandler(500)
|
||||
@@ -325,13 +652,21 @@ def create_app():
|
||||
# Roll back any failed database session
|
||||
db.session.rollback()
|
||||
|
||||
if request.path.startswith('/users/disponibilities') or \
|
||||
request.path.startswith('/users/api/'):
|
||||
return jsonify({
|
||||
'error': 'Internal server error',
|
||||
'message': 'An unexpected error occurred. Please try again later.'
|
||||
}), 500
|
||||
return render_template('errors/500.html'), 500
|
||||
# The id is the only thing that connects a user saying "it broke when
|
||||
# I clicked save" to the stack trace in errors.log. It identifies one
|
||||
# request and nothing else — no session, no account, nothing an
|
||||
# attacker can use — so showing it costs nothing (OBS-005).
|
||||
request_id = g.get('request_id', '-')
|
||||
|
||||
if wants_json_response():
|
||||
return jsonify(
|
||||
{
|
||||
'error': 'Internal server error',
|
||||
'message': 'An unexpected error occurred. Please try again later.',
|
||||
'request_id': request_id,
|
||||
}
|
||||
), 500
|
||||
return render_template('errors/500.html', request_id=request_id), 500
|
||||
|
||||
@app.errorhandler(HTTPException)
|
||||
def handle_http_exception(error):
|
||||
@@ -343,13 +678,10 @@ def create_app():
|
||||
Returns:
|
||||
Response: JSON error for API, re-raises for others.
|
||||
"""
|
||||
if request.path.startswith('/users/disponibilities') or \
|
||||
request.path.startswith('/users/api/'):
|
||||
return jsonify({
|
||||
'error': error.name,
|
||||
'message': error.description,
|
||||
'code': error.code
|
||||
}), error.code
|
||||
if wants_json_response():
|
||||
return jsonify(
|
||||
{'error': error.name, 'message': error.description, 'code': error.code}
|
||||
), error.code
|
||||
return error
|
||||
|
||||
# =========================================================================
|
||||
@@ -357,25 +689,40 @@ def create_app():
|
||||
# =========================================================================
|
||||
with app.app_context():
|
||||
import app.models as models # noqa: F401 — registers all models with SQLAlchemy
|
||||
db.create_all()
|
||||
|
||||
# NOTE: create_all() only ever creates missing tables. It never adds a
|
||||
# column to an existing one, so a model change is silently absent from
|
||||
# any database that already has the table. Replacing this with Alembic
|
||||
# is tracked as DB-002/DB-004; until then the behaviour is preserved.
|
||||
if app.config['AUTO_CREATE_TABLES']:
|
||||
db.create_all()
|
||||
|
||||
# Start the Discord bot for notifications
|
||||
try:
|
||||
from app.discord_bot import start_bot
|
||||
start_bot(flask_app=app)
|
||||
except Exception as e:
|
||||
app.logger.warning('Could not start Discord bot: %s', e)
|
||||
if app.config['ENABLE_DISCORD_BOT']:
|
||||
try:
|
||||
from app.discord_bot import start_bot
|
||||
|
||||
start_bot(flask_app=app)
|
||||
except Exception: # the site must come up even if the bot cannot
|
||||
# With the message alone, the two ways this fails — a bad token
|
||||
# and a broken import in discord_bot — read identically, and
|
||||
# neither is diagnosable from one line. Notifications are down
|
||||
# either way, so the traceback is the whole value of the log.
|
||||
# Error, not warning: a club that receives no reminders has lost
|
||||
# a feature, and the old level put that next to the deprecation
|
||||
# notices.
|
||||
app.logger.error(
|
||||
'Could not start the Discord bot. The site is up; no notification '
|
||||
'will be sent until this is fixed.',
|
||||
exc_info=True,
|
||||
)
|
||||
|
||||
return app
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
# Only used for development - production uses wsgi.py (Waitress)
|
||||
app = create_app()
|
||||
debug_mode = os.getenv('FLASK_DEBUG', 'false').lower() == 'true'
|
||||
if debug_mode:
|
||||
app.logger.warning(
|
||||
'Running in DEBUG mode with Flask built-in server. '
|
||||
'This is NOT suitable for production. Use wsgi.py instead.'
|
||||
)
|
||||
app.run(debug=debug_mode, host='0.0.0.0', port=10000)
|
||||
# No __main__ block here on purpose. There used to be one, and with run.py
|
||||
# and wsgi.py that made three ways to start the application, each with its
|
||||
# own host, port and debug default — `python app/app.py` bound 0.0.0.0:10000
|
||||
# while `python run.py` bound 127.0.0.2:5000 with the debugger on. This
|
||||
# module defines the factory; run.py starts it for development, wsgi.py for
|
||||
# production (ARCH-007).
|
||||
Reference in New Issue
Block a user