fix(web): une erreur sur un point JSON ne renvoie plus une page HTML
STD-09, trouve en recroisant l'audit anterieur -- celui mene sur le miroir GitHub, jamais repasse depuis qu'on a decouvert que ce n'etait pas la bonne source. Sept gestionnaires d'erreur portaient chacun leur copie d'une liste de prefixes d'URL decidant "JSON ou page HTML". Les copies avaient derive -- trois testaient /users/coach-availability, quatre non -- et toutes manquaient les memes points. Un fetch() qui recoit une page d'erreur HTML leve en la parsant : sur le calendrier, les listes de selections et d'equipes restaient vides, sans message dans la page et sans rien dans le journal. Deux choses apprises en ecrivant le test, aucune n'etait dans le constat. L'approche par prefixe ne pouvait pas etre reparee. Trois des seize vues JSON sont a des chemins qu'aucun prefixe ne distingue des pages HTML voisines -- /matches/<id>/toggle-presence/<id> et ses deux cousins, que les gabarits appellent justement en fetch(). Les vues se declarent donc elles-memes (@json_endpoint, app/api.py), et un test parcourt la carte des URL pour verifier qu'aucune vue appelant jsonify n'a ete oubliee. Et surtout : @login_required n'atteint jamais le gestionnaire 401. Flask-Login intercepte avant et redirige. Les seize points JSON repondaient donc a une session expiree par une 302 vers un formulaire HTML, quoi que dise la liste de prefixes. Reecrire la liste seule aurait eu l'air d'un correctif sans rien changer. Au passage, le message flash de ce gestionnaire etait la seule chaine de l'application qui n'avait jamais ete traduite. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
GGThed
+87
-35
@@ -9,7 +9,18 @@ import secrets
|
||||
|
||||
import markupsafe
|
||||
from dotenv import load_dotenv
|
||||
from flask import Flask, g, jsonify, redirect, render_template, request, url_for
|
||||
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
|
||||
@@ -19,6 +30,38 @@ from app.extensions import babel, csrf, db, limiter, login_manager
|
||||
|
||||
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
|
||||
|
||||
|
||||
def nl2br(value):
|
||||
"""Convert newlines to HTML line breaks.
|
||||
@@ -482,6 +525,36 @@ def create_app(config=None):
|
||||
# =========================================================================
|
||||
# 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.
|
||||
@@ -492,32 +565,21 @@ def create_app(config=None):
|
||||
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):
|
||||
@@ -529,9 +591,7 @@ def create_app(config=None):
|
||||
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
|
||||
|
||||
@@ -545,9 +605,7 @@ def create_app(config=None):
|
||||
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
|
||||
|
||||
@@ -561,9 +619,7 @@ def create_app(config=None):
|
||||
Returns:
|
||||
Response: JSON error for API or rendered page.
|
||||
"""
|
||||
if request.path.startswith('/users/disponibilities') or request.path.startswith(
|
||||
'/users/api/'
|
||||
):
|
||||
if wants_json_response():
|
||||
return jsonify(
|
||||
{'error': 'Too many requests', 'message': 'Please try again later.'}
|
||||
), 429
|
||||
@@ -593,9 +649,7 @@ def create_app(config=None):
|
||||
# attacker can use — so showing it costs nothing (OBS-005).
|
||||
request_id = g.get('request_id', '-')
|
||||
|
||||
if request.path.startswith('/users/disponibilities') or request.path.startswith(
|
||||
'/users/api/'
|
||||
):
|
||||
if wants_json_response():
|
||||
return jsonify(
|
||||
{
|
||||
'error': 'Internal server error',
|
||||
@@ -615,9 +669,7 @@ def create_app(config=None):
|
||||
Returns:
|
||||
Response: JSON error for API, re-raises for others.
|
||||
"""
|
||||
if request.path.startswith('/users/disponibilities') or request.path.startswith(
|
||||
'/users/api/'
|
||||
):
|
||||
if wants_json_response():
|
||||
return jsonify(
|
||||
{'error': error.name, 'message': error.description, 'code': error.code}
|
||||
), error.code
|
||||
|
||||
Reference in New Issue
Block a user