diff --git a/app/app.py b/app/app.py index 72085d1..7d00949 100644 --- a/app/app.py +++ b/app/app.py @@ -249,6 +249,27 @@ def create_app(config=None): # 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. @@ -533,6 +554,12 @@ def create_app(config=None): # Roll back any failed database session db.session.rollback() + # 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 request.path.startswith('/users/disponibilities') or request.path.startswith( '/users/api/' ): @@ -540,9 +567,10 @@ def create_app(config=None): { 'error': 'Internal server error', 'message': 'An unexpected error occurred. Please try again later.', + 'request_id': request_id, } ), 500 - return render_template('errors/500.html'), 500 + return render_template('errors/500.html', request_id=request_id), 500 @app.errorhandler(HTTPException) def handle_http_exception(error): diff --git a/app/logging_config.py b/app/logging_config.py index 2dd0616..82397e3 100644 --- a/app/logging_config.py +++ b/app/logging_config.py @@ -14,6 +14,37 @@ import os import re from logging.handlers import RotatingFileHandler +#: Value used when a record is emitted outside a request — startup, the +#: Discord bot thread, the scheduler. Short and obviously not an id, so a +#: grep for one never matches it by accident. +NO_REQUEST = '-' + + +class RequestIdFilter(logging.Filter): + """Stamp every record with the id of the request that produced it. + + Without this, a 500 in errors.log and the six lines in app.log that led + to it are related only by their timestamps, which is not a relation when + the server is handling more than one request at a time (OBS-005). + + The id is generated per request and never read from an inbound header. + Accepting one would be convenient for tracing across nginx, and it would + also let any caller write arbitrary text — newlines included — into the + log file, which is how a log gets forged rather than read. There is no + trusted proxy to take it from while OPS-002 is open. + """ + + def filter(self, record): + record.request_id = NO_REQUEST + try: + from flask import g, has_request_context + + if has_request_context(): + record.request_id = g.get('request_id', NO_REQUEST) + except Exception: # noqa: BLE001 — logging must never be the thing that fails + pass + return True + class SensitiveDataFilter(logging.Filter): """Logging filter that redacts sensitive information from log messages. @@ -95,10 +126,17 @@ def configure_logging(app): # Create the sensitive data filter sensitive_filter = SensitiveDataFilter() + request_id_filter = RequestIdFilter() - # Formatter with timestamp, level, module, and message + # Formatter with timestamp, level, module, request id, and message. + # + # request_id comes from RequestIdFilter, which is attached to every + # handler below. A handler that formats with this string and does not + # carry the filter raises on its first record — so if one is ever added, + # add the filter with it. formatter = logging.Formatter( - '[%(asctime)s] %(levelname)s [%(name)s:%(lineno)d] %(message)s', datefmt='%Y-%m-%d %H:%M:%S' + '[%(asctime)s] %(levelname)s [%(name)s:%(lineno)d] [%(request_id)s] %(message)s', + datefmt='%Y-%m-%d %H:%M:%S', ) # ------------------------------------------------------------------------- @@ -112,6 +150,7 @@ def configure_logging(app): error_handler.setLevel(logging.ERROR) error_handler.setFormatter(formatter) error_handler.addFilter(sensitive_filter) + error_handler.addFilter(request_id_filter) app.logger.addHandler(error_handler) # ------------------------------------------------------------------------- @@ -125,6 +164,7 @@ def configure_logging(app): auth_handler.setLevel(logging.INFO) auth_handler.setFormatter(formatter) auth_handler.addFilter(sensitive_filter) + auth_handler.addFilter(request_id_filter) # Create a named logger specifically for auth events auth_logger = logging.getLogger('team_tryouts.auth') @@ -143,6 +183,7 @@ def configure_logging(app): app_handler.setLevel(log_level) app_handler.setFormatter(formatter) app_handler.addFilter(sensitive_filter) + app_handler.addFilter(request_id_filter) app.logger.addHandler(app_handler) # ------------------------------------------------------------------------- @@ -156,6 +197,7 @@ def configure_logging(app): console_handler.setLevel(logging.DEBUG if debug_mode else log_level) console_handler.setFormatter(formatter) console_handler.addFilter(sensitive_filter) + console_handler.addFilter(request_id_filter) app.logger.addHandler(console_handler) # ------------------------------------------------------------------------- diff --git a/app/templates/errors/500.html b/app/templates/errors/500.html index ffaab59..2970cde 100644 --- a/app/templates/errors/500.html +++ b/app/templates/errors/500.html @@ -8,6 +8,12 @@

{{ _('500 — Internal Server Error') }}

{{ _('Something went wrong on our end. The error has been logged and will be investigated. Please try again later.') }}

+ {# The reference is what makes a report actionable: it names one request + in errors.log. It identifies nothing else — no session, no account — + so there is nothing to protect here (OBS-005). #} + {% if request_id and request_id != '-' %} +

{{ _('Reference to quote if you report this:') }} {{ request_id }}

+ {% endif %} {{ _('Try Again') }} diff --git a/app/templates/layouts/base.html b/app/templates/layouts/base.html index 9e3e992..c7279f7 100644 --- a/app/templates/layouts/base.html +++ b/app/templates/layouts/base.html @@ -201,6 +201,20 @@
{% include "layouts/_language_switcher.html" %}
+ {# The same `content` block as the signed-in branch, rendered + here too — `self.content()` rather than a second + `{% block %}`, which Jinja refuses. + + The error pages (400, 403, 404, 429, 500) all fill `content`, + and it existed only inside the `is_authenticated` branch: a + signed-out visitor hitting any of them got the logo, the + language switcher and no message whatsoever. The still + said "404", which is most of why nobody noticed. + + Only one branch of the `if` runs, so this never double-renders. + Sign-in pages fill `auth_content` instead and leave this + empty. #} + {{ self.content() }} {% block auth_content %}{% endblock %} </div> </div> diff --git a/app/translations/en/LC_MESSAGES/messages.mo b/app/translations/en/LC_MESSAGES/messages.mo index 281ff22..f62402f 100644 Binary files a/app/translations/en/LC_MESSAGES/messages.mo and b/app/translations/en/LC_MESSAGES/messages.mo differ diff --git a/app/translations/en/LC_MESSAGES/messages.po b/app/translations/en/LC_MESSAGES/messages.po index 0fce2b3..4055819 100644 --- a/app/translations/en/LC_MESSAGES/messages.po +++ b/app/translations/en/LC_MESSAGES/messages.po @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: team-tryouts VERSION\n" "Report-Msgid-Bugs-To: EMAIL@ADDRESS\n" -"POT-Creation-Date: 2026-08-11 13:32-0400\n" +"POT-Creation-Date: 2026-08-11 15:25-0400\n" "PO-Revision-Date: 2026-08-07 20:22-0400\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language: en\n" @@ -20,7 +20,7 @@ msgstr "" "Generated-By: Babel 2.18.0\n" #: app/forms.py:37 app/routes/auth.py:224 app/routes/auth.py:374 -#: app/routes/users/contracts.py:95 +#: app/routes/users/contracts.py:96 #, python-format msgid "%(field)s: %(msg)s" msgstr "%(field)s: %(msg)s" @@ -633,11 +633,11 @@ msgstr "User %(full_name)s created as %(role)s!" msgid "Only coaches can manage availability." msgstr "Only coaches can manage availability." -#: app/routes/users/contracts.py:83 +#: app/routes/users/contracts.py:84 msgid "Only presidents, managers, and coaches can upload contracts." msgstr "Only presidents, managers, and coaches can upload contracts." -#: app/routes/users/contracts.py:102 +#: app/routes/users/contracts.py:103 msgid "You do not have permission to upload a contract for this player." msgstr "You do not have permission to upload a contract for this player." @@ -650,15 +650,15 @@ msgstr "Contract uploaded successfully for %(username)s!" msgid "Only the player can upload their signed contract." msgstr "Only the player can upload their signed contract." -#: app/routes/users/contracts.py:176 +#: app/routes/users/contracts.py:175 msgid "Signed contract uploaded successfully!" msgstr "Signed contract uploaded successfully!" -#: app/routes/users/contracts.py:186 app/routes/users/contracts.py:199 +#: app/routes/users/contracts.py:185 app/routes/users/contracts.py:200 msgid "You do not have permission to download this contract." msgstr "You do not have permission to download this contract." -#: app/routes/users/contracts.py:202 +#: app/routes/users/contracts.py:203 msgid "No signed contract available." msgstr "No signed contract available." @@ -881,7 +881,11 @@ msgstr "" "Something went wrong on our end. The error has been logged and will be " "investigated. Please try again later." -#: app/templates/errors/500.html:12 +#: app/templates/errors/500.html:15 +msgid "Reference to quote if you report this:" +msgstr "Reference to quote if you report this:" + +#: app/templates/errors/500.html:18 msgid "Try Again" msgstr "Try Again" @@ -889,78 +893,78 @@ msgstr "Try Again" msgid "Language" msgstr "Language" -#: app/templates/layouts/base.html:33 app/templates/layouts/base.html:139 +#: app/templates/layouts/base.html:48 app/templates/layouts/base.html:154 #: app/templates/pages/dashboard.html:2 app/templates/pages/dashboard.html:3 msgid "Dashboard" msgstr "Dashboard" -#: app/templates/layouts/base.html:39 app/templates/pages/tryouts.html:2 +#: app/templates/layouts/base.html:54 app/templates/pages/tryouts.html:2 #: app/templates/pages/tryouts.html:3 msgid "Tryouts" msgstr "Tryouts" -#: app/templates/layouts/base.html:45 app/templates/pages/calendar.html:2 +#: app/templates/layouts/base.html:60 app/templates/pages/calendar.html:2 #: app/templates/pages/calendar.html:3 msgid "Calendar" msgstr "Calendar" -#: app/templates/layouts/base.html:52 app/templates/pages/dashboard.html:43 +#: app/templates/layouts/base.html:67 app/templates/pages/dashboard.html:43 #: app/templates/pages/evaluations.html:2 #: app/templates/pages/evaluations.html:3 msgid "Evaluations" msgstr "Evaluations" -#: app/templates/layouts/base.html:60 app/templates/pages/my_teams.html:2 +#: app/templates/layouts/base.html:75 app/templates/pages/my_teams.html:2 #: app/templates/pages/my_teams.html:3 msgid "My Team(s)" msgstr "My Team(s)" -#: app/templates/layouts/base.html:67 +#: app/templates/layouts/base.html:82 msgid "Manage Teams" msgstr "Manage Teams" -#: app/templates/layouts/base.html:75 app/templates/pages/users.html:2 +#: app/templates/layouts/base.html:90 app/templates/pages/users.html:2 #: app/templates/pages/users.html:3 msgid "Manage Users" msgstr "Manage Users" -#: app/templates/layouts/base.html:83 +#: app/templates/layouts/base.html:98 #: app/templates/pages/player_personal_notes.html:2 #: app/templates/pages/player_personal_notes.html:3 msgid "My Notes" msgstr "My Notes" -#: app/templates/layouts/base.html:91 +#: app/templates/layouts/base.html:106 msgid "Availability" msgstr "Availability" -#: app/templates/layouts/base.html:97 +#: app/templates/layouts/base.html:112 msgid "Notes & One on One" msgstr "Notes & One on One" -#: app/templates/layouts/base.html:104 app/templates/pages/contracts.html:2 +#: app/templates/layouts/base.html:119 app/templates/pages/contracts.html:2 #: app/templates/pages/contracts.html:3 app/templates/pages/profile.html:9 msgid "Contracts" msgstr "Contracts" -#: app/templates/layouts/base.html:111 app/templates/pages/profile.html:2 +#: app/templates/layouts/base.html:126 app/templates/pages/profile.html:2 #: app/templates/pages/profile.html:3 msgid "My Profile" msgstr "My Profile" -#: app/templates/layouts/base.html:122 +#: app/templates/layouts/base.html:137 msgid "Logout" msgstr "Logout" -#: app/templates/layouts/base.html:143 +#: app/templates/layouts/base.html:158 msgid "Toggle dark mode" msgstr "Toggle dark mode" -#: app/templates/layouts/base.html:155 app/templates/layouts/base.html:174 +#: app/templates/layouts/base.html:170 app/templates/layouts/base.html:189 msgid "Dismiss" msgstr "Dismiss" -#: app/templates/layouts/base.html:184 +#: app/templates/layouts/base.html:199 msgid "Team Tryout Management System" msgstr "Team Tryout Management System" diff --git a/app/translations/fr/LC_MESSAGES/messages.mo b/app/translations/fr/LC_MESSAGES/messages.mo index 5d78727..e520e0a 100644 Binary files a/app/translations/fr/LC_MESSAGES/messages.mo and b/app/translations/fr/LC_MESSAGES/messages.mo differ diff --git a/app/translations/fr/LC_MESSAGES/messages.po b/app/translations/fr/LC_MESSAGES/messages.po index 89827a1..b1dda48 100644 --- a/app/translations/fr/LC_MESSAGES/messages.po +++ b/app/translations/fr/LC_MESSAGES/messages.po @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: team-tryouts VERSION\n" "Report-Msgid-Bugs-To: EMAIL@ADDRESS\n" -"POT-Creation-Date: 2026-08-11 13:32-0400\n" +"POT-Creation-Date: 2026-08-11 15:25-0400\n" "PO-Revision-Date: 2026-08-07 20:22-0400\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language: fr\n" @@ -20,7 +20,7 @@ msgstr "" "Generated-By: Babel 2.18.0\n" #: app/forms.py:37 app/routes/auth.py:224 app/routes/auth.py:374 -#: app/routes/users/contracts.py:95 +#: app/routes/users/contracts.py:96 #, python-format msgid "%(field)s: %(msg)s" msgstr "%(field)s : %(msg)s" @@ -639,11 +639,11 @@ msgstr "Utilisateur %(full_name)s créé avec le rôle %(role)s." msgid "Only coaches can manage availability." msgstr "Seuls les coachs peuvent gérer leurs disponibilités." -#: app/routes/users/contracts.py:83 +#: app/routes/users/contracts.py:84 msgid "Only presidents, managers, and coaches can upload contracts." msgstr "Seuls les présidents, gérants et coachs peuvent téléverser un contrat." -#: app/routes/users/contracts.py:102 +#: app/routes/users/contracts.py:103 msgid "You do not have permission to upload a contract for this player." msgstr "Vous n’avez pas les droits pour téléverser un contrat pour ce joueur." @@ -656,15 +656,15 @@ msgstr "Contrat téléversé pour %(username)s." msgid "Only the player can upload their signed contract." msgstr "Seul le joueur peut téléverser son contrat signé." -#: app/routes/users/contracts.py:176 +#: app/routes/users/contracts.py:175 msgid "Signed contract uploaded successfully!" msgstr "Contrat signé téléversé." -#: app/routes/users/contracts.py:186 app/routes/users/contracts.py:199 +#: app/routes/users/contracts.py:185 app/routes/users/contracts.py:200 msgid "You do not have permission to download this contract." msgstr "Vous n’avez pas les droits pour télécharger ce contrat." -#: app/routes/users/contracts.py:202 +#: app/routes/users/contracts.py:203 msgid "No signed contract available." msgstr "Aucun contrat signé disponible." @@ -887,7 +887,11 @@ msgstr "" "Une erreur est survenue de notre côté. Elle a été journalisée et sera " "examinée. Veuillez réessayer dans un instant." -#: app/templates/errors/500.html:12 +#: app/templates/errors/500.html:15 +msgid "Reference to quote if you report this:" +msgstr "Référence à indiquer si vous signalez ce problème :" + +#: app/templates/errors/500.html:18 msgid "Try Again" msgstr "Réessayer" @@ -895,78 +899,78 @@ msgstr "Réessayer" msgid "Language" msgstr "Langue" -#: app/templates/layouts/base.html:33 app/templates/layouts/base.html:139 +#: app/templates/layouts/base.html:48 app/templates/layouts/base.html:154 #: app/templates/pages/dashboard.html:2 app/templates/pages/dashboard.html:3 msgid "Dashboard" msgstr "Tableau de bord" -#: app/templates/layouts/base.html:39 app/templates/pages/tryouts.html:2 +#: app/templates/layouts/base.html:54 app/templates/pages/tryouts.html:2 #: app/templates/pages/tryouts.html:3 msgid "Tryouts" msgstr "Sélections" -#: app/templates/layouts/base.html:45 app/templates/pages/calendar.html:2 +#: app/templates/layouts/base.html:60 app/templates/pages/calendar.html:2 #: app/templates/pages/calendar.html:3 msgid "Calendar" msgstr "Calendrier" -#: app/templates/layouts/base.html:52 app/templates/pages/dashboard.html:43 +#: app/templates/layouts/base.html:67 app/templates/pages/dashboard.html:43 #: app/templates/pages/evaluations.html:2 #: app/templates/pages/evaluations.html:3 msgid "Evaluations" msgstr "Évaluations" -#: app/templates/layouts/base.html:60 app/templates/pages/my_teams.html:2 +#: app/templates/layouts/base.html:75 app/templates/pages/my_teams.html:2 #: app/templates/pages/my_teams.html:3 msgid "My Team(s)" msgstr "Mon ou mes équipes" -#: app/templates/layouts/base.html:67 +#: app/templates/layouts/base.html:82 msgid "Manage Teams" msgstr "Gestion des équipes" -#: app/templates/layouts/base.html:75 app/templates/pages/users.html:2 +#: app/templates/layouts/base.html:90 app/templates/pages/users.html:2 #: app/templates/pages/users.html:3 msgid "Manage Users" msgstr "Gestion des utilisateurs" -#: app/templates/layouts/base.html:83 +#: app/templates/layouts/base.html:98 #: app/templates/pages/player_personal_notes.html:2 #: app/templates/pages/player_personal_notes.html:3 msgid "My Notes" msgstr "Mes notes" -#: app/templates/layouts/base.html:91 +#: app/templates/layouts/base.html:106 msgid "Availability" msgstr "Disponibilités" -#: app/templates/layouts/base.html:97 +#: app/templates/layouts/base.html:112 msgid "Notes & One on One" msgstr "Notes et rencontres individuelles" -#: app/templates/layouts/base.html:104 app/templates/pages/contracts.html:2 +#: app/templates/layouts/base.html:119 app/templates/pages/contracts.html:2 #: app/templates/pages/contracts.html:3 app/templates/pages/profile.html:9 msgid "Contracts" msgstr "Contrats" -#: app/templates/layouts/base.html:111 app/templates/pages/profile.html:2 +#: app/templates/layouts/base.html:126 app/templates/pages/profile.html:2 #: app/templates/pages/profile.html:3 msgid "My Profile" msgstr "Mon profil" -#: app/templates/layouts/base.html:122 +#: app/templates/layouts/base.html:137 msgid "Logout" msgstr "Déconnexion" -#: app/templates/layouts/base.html:143 +#: app/templates/layouts/base.html:158 msgid "Toggle dark mode" msgstr "Basculer le mode sombre" -#: app/templates/layouts/base.html:155 app/templates/layouts/base.html:174 +#: app/templates/layouts/base.html:170 app/templates/layouts/base.html:189 msgid "Dismiss" msgstr "Fermer" -#: app/templates/layouts/base.html:184 +#: app/templates/layouts/base.html:199 msgid "Team Tryout Management System" msgstr "Système de gestion des sélections d’équipe" diff --git a/tests/test_request_id.py b/tests/test_request_id.py new file mode 100644 index 0000000..24d53b3 --- /dev/null +++ b/tests/test_request_id.py @@ -0,0 +1,214 @@ +"""Every request has a name, and its log lines carry it (OBS-005). + +Before this, a 500 in errors.log and the lines in app.log that led to it were +related only by their timestamps — which is not a relation once the server is +handling more than one request at a time. And a user saying "it broke when I +clicked save" gave nobody anything to grep for. + +The id is deliberately generated, never read from an inbound header. That is +the test worth reading in this file: accepting one would be convenient for +tracing through nginx, and would also let any caller write arbitrary text — +newlines included — into the log, which is how a log stops being evidence. +""" + +import logging +import re + +import pytest + +from app.logging_config import NO_REQUEST, RequestIdFilter + +ID_PATTERN = re.compile(r'^[0-9a-f]{16}$') + + +class TestTheHeader: + def test_every_response_carries_one(self, client): + response = client.get('/auth/login') + + assert ID_PATTERN.match(response.headers['X-Request-Id']) + + def test_two_requests_get_different_ids(self, client): + first = client.get('/auth/login').headers['X-Request-Id'] + second = client.get('/auth/login').headers['X-Request-Id'] + + assert first != second + + def test_an_error_response_carries_one_too(self, client): + """The case it exists for.""" + response = client.get('/no-such-page') + + assert response.status_code == 404 + assert ID_PATTERN.match(response.headers['X-Request-Id']) + + +class TestInboundHeadersAreIgnored: + """The security property, not a convenience. + + Waitress currently runs with trusted_proxy='*' (OPS-002), so anything in + an inbound header comes from whoever sent the request. + """ + + def test_a_supplied_id_is_not_adopted(self, client): + response = client.get('/auth/login', headers={'X-Request-Id': 'chosen-by-the-caller'}) + + assert response.headers['X-Request-Id'] != 'chosen-by-the-caller' + assert ID_PATTERN.match(response.headers['X-Request-Id']) + + def test_the_id_is_always_hex_so_it_cannot_forge_a_log_line(self, client): + """The property that makes log injection impossible. + + A value carrying a newline writes a second line that looks exactly + like a real log entry — that is how a log stops being evidence. + Since the id is generated from a fixed alphabet rather than taken + from the request, no input reaches the log through this field at all. + + Asserted on the alphabet rather than by sending a newline: Werkzeug's + test client refuses to send such a header, so the attack cannot even + be constructed through it — which proves nothing about the app. + """ + for supplied in ('../../etc/passwd', 'a b c', '<script>', 'x' * 500): + value = client.get('/auth/login', headers={'X-Request-Id': supplied}).headers[ + 'X-Request-Id' + ] + assert ID_PATTERN.match(value), f'{supplied!r} influenced the id' + + +class TestTheFilter: + """RequestIdFilter has to be safe on records emitted from anywhere. + + The Discord bot logs from its own thread, the scheduler from another, and + configure_logging runs before any request exists. A filter that raised + there would take the log down with it. + """ + + @pytest.fixture + def record(self): + return logging.LogRecord('app.test', logging.INFO, __file__, 1, 'hello', None, None) + + def test_outside_a_request_the_record_still_gets_a_value(self, record): + assert RequestIdFilter().filter(record) is True + assert record.request_id == NO_REQUEST + + def test_inside_a_request_it_gets_that_request_id(self, app, record): + with app.test_request_context('/'): + from flask import g + + g.request_id = 'abcdef0123456789' + RequestIdFilter().filter(record) + + assert record.request_id == 'abcdef0123456789' + + def test_a_request_without_the_before_request_hook_does_not_crash(self, app, record): + """g exists but the key does not — the case during app teardown, and + in any test that pushes a bare context.""" + with app.test_request_context('/'): + RequestIdFilter().filter(record) + + assert record.request_id == NO_REQUEST + + def test_the_formatter_never_raises_for_want_of_the_field(self, app, record, caplog): + """The formatter references %(request_id)s. A handler carrying that + format without this filter raises on its first record — which would + turn a logged error into a crash inside the error handler.""" + formatter = logging.Formatter('[%(request_id)s] %(message)s') + RequestIdFilter().filter(record) + + assert formatter.format(record) == f'[{NO_REQUEST}] hello' + + +class TestTheErrorPage: + """Driven through a real failing request rather than by rendering the + template: the id has to survive the whole path — before_request, the + handler, the template — and rendering the file directly would skip all + three.""" + + @pytest.fixture + def exploding_app(self, app): + app.config['PROPAGATE_EXCEPTIONS'] = False + + @app.route('/tests/boom') + def boom(): + raise RuntimeError('deliberate') + + return app + + def test_the_five_hundred_page_quotes_the_reference(self, exploding_app): + """Without it on the page, a user report cannot be tied to a trace.""" + client = exploding_app.test_client() + + response = client.get('/tests/boom') + + assert response.status_code == 500 + page = response.get_data(as_text=True) + assert response.headers['X-Request-Id'] in page + + def test_a_signed_out_visitor_sees_the_page_at_all(self, exploding_app): + """The error templates fill `content`, which used to exist only in + the signed-in branch of the layout: anonymous visitors got the logo + and nothing else, while the <title> still said 500.""" + page = exploding_app.test_client().get('/tests/boom').get_data(as_text=True) + + assert 'error-container' in page + + def test_the_json_paths_carry_it_too(self, exploding_app): + """A fetch that 500s gets the same reference, or the JavaScript half + of the application is untraceable.""" + + @exploding_app.route('/users/api/boom') + def api_boom(): + raise RuntimeError('deliberate') + + response = exploding_app.test_client().get('/users/api/boom') + + assert response.status_code == 500 + assert response.get_json()['request_id'] == response.headers['X-Request-Id'] + + +class TestErrorPagesSpeakToEveryone: + """Every error page must say something, signed in or not (found while + adding the reference above). + + The failure mode is quiet by construction: the <title> comes from a block + outside the branch, so the tab said "404 — Page Not Found" over a page + that carried no message. The status code was right, the logs were right, + and the page was blank. + """ + + ERRORS = { + 400: '/tests/error/400', + 403: '/tests/error/403', + 404: '/no-such-page-anywhere', + 500: '/tests/error/500', + } + + @pytest.fixture + def erroring_app(self, app): + from flask import abort + + app.config['PROPAGATE_EXCEPTIONS'] = False + + @app.route('/tests/error/<int:code>') + def raise_error(code): + if code == 500: + raise RuntimeError('deliberate') + abort(code) + + return app + + @pytest.mark.parametrize('code', sorted(ERRORS)) + def test_a_signed_out_visitor_gets_the_message(self, erroring_app, code): + response = erroring_app.test_client().get(self.ERRORS[code]) + + assert response.status_code == code + page = response.get_data(as_text=True) + assert 'error-container' in page, f'{code} renders no body for a signed-out visitor' + + @pytest.mark.parametrize('code', sorted(ERRORS)) + def test_a_signed_in_visitor_gets_it_once_and_not_twice(self, erroring_app, as_role, code): + """`self.content()` sits in the other branch of the same `if`, so it + can never double-render — asserted rather than assumed.""" + as_role('player') + + page = erroring_app.test_client().get(self.ERRORS[code]).get_data(as_text=True) + + assert page.count('error-container') <= 1