fix: worker report views survive stale/typo'd ?project= and ?team= params
All three worker batch-report views (HTML / CSV / PDF) passed raw query-string values into queryset filters — ?team=abc raised ValueError deep in the ORM, and the PDF view's display-name lookups raised Project/Team.DoesNotExist on a deleted id (stale bookmark) → 500s. New _int_param_or_none() sanitizer coerces filter params at the view boundary (junk degrades to 'no filter'); the PDF's display-name lookups fall back to 'All Projects'/'All Teams' on DoesNotExist. Template dropdowns already compare via |stringformat so int params are safe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
4d029dd6e5
commit
f0f3938621
@ -3862,3 +3862,42 @@ class AttendanceDateRangeCapTests(TestCase):
|
||||
def test_range_of_exactly_31_days_is_allowed(self):
|
||||
errs = self._range_errors('2026-01-01', '2026-02-01') # 31-day diff
|
||||
self.assertFalse(any('31 days' in e for e in errs), errs)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# === AUDIT FIX #6 — WORKER REPORT VIEWS MUST SURVIVE BAD QUERY PARAMS ===
|
||||
# Stale bookmarks and hand-edited URLs (?project=99999 after a delete,
|
||||
# ?team=abc from a typo) hit all three worker-report views (HTML / CSV /
|
||||
# PDF). Bad filter values must degrade to "no filter", never 500.
|
||||
# render_to_pdf is mocked so these tests don't depend on the local GTK3
|
||||
# install — the crashes under test happen BEFORE PDF rendering anyway.
|
||||
# =============================================================================
|
||||
|
||||
class WorkerReportBadParamsTests(TestCase):
|
||||
"""?project= / ?team= junk degrades gracefully on all 3 report views."""
|
||||
|
||||
def setUp(self):
|
||||
self.admin = User.objects.create_user(
|
||||
username='pdfguard_admin', password='x', is_staff=True)
|
||||
self.client.force_login(self.admin)
|
||||
|
||||
def _get_pdf(self, params):
|
||||
with mock.patch('core.utils.render_to_pdf',
|
||||
return_value=b'%PDF-1.4 fake'):
|
||||
return self.client.get(reverse('worker_batch_report_pdf'), params)
|
||||
|
||||
def test_pdf_nonexistent_project_id_falls_back(self):
|
||||
resp = self._get_pdf({'project': '99999'})
|
||||
self.assertEqual(resp.status_code, 200)
|
||||
|
||||
def test_pdf_non_numeric_team_falls_back(self):
|
||||
resp = self._get_pdf({'team': 'abc'})
|
||||
self.assertEqual(resp.status_code, 200)
|
||||
|
||||
def test_html_non_numeric_project_falls_back(self):
|
||||
resp = self.client.get(reverse('worker_batch_report'), {'project': 'abc'})
|
||||
self.assertEqual(resp.status_code, 200)
|
||||
|
||||
def test_csv_non_numeric_team_falls_back(self):
|
||||
resp = self.client.get(reverse('worker_batch_report_csv'), {'team': 'abc'})
|
||||
self.assertEqual(resp.status_code, 200)
|
||||
|
||||
@ -1645,6 +1645,19 @@ def worker_edit(request, worker_id=None):
|
||||
# === WORKER BATCH REPORT ===
|
||||
# =============================================================
|
||||
|
||||
def _int_param_or_none(raw):
|
||||
"""Coerce a raw query-string value to int, or None if absent/invalid.
|
||||
|
||||
Stale bookmarks and hand-edited URLs (?project=abc, ?team= after a
|
||||
delete) must degrade to "no filter" — never crash the view with a
|
||||
ValueError deep inside a queryset. (Audit fix, Jun 2026.)
|
||||
"""
|
||||
try:
|
||||
return int(raw)
|
||||
except (TypeError, ValueError):
|
||||
return None
|
||||
|
||||
|
||||
def _build_worker_report_context(status=None, project_id=None, team_id=None):
|
||||
"""Build the per-worker aggregation list used by HTML / CSV / PDF views.
|
||||
|
||||
@ -1724,8 +1737,8 @@ def worker_batch_report(request):
|
||||
return HttpResponseForbidden("Admin access required.")
|
||||
|
||||
status = request.GET.get('status') or 'all'
|
||||
project_id = request.GET.get('project') or None
|
||||
team_id = request.GET.get('team') or None
|
||||
project_id = _int_param_or_none(request.GET.get('project'))
|
||||
team_id = _int_param_or_none(request.GET.get('team'))
|
||||
|
||||
rows = _build_worker_report_context(status=status, project_id=project_id, team_id=team_id)
|
||||
|
||||
@ -1749,8 +1762,8 @@ def worker_batch_report_csv(request):
|
||||
return HttpResponseForbidden("Admin access required.")
|
||||
|
||||
status = request.GET.get('status') or 'all'
|
||||
project_id = request.GET.get('project') or None
|
||||
team_id = request.GET.get('team') or None
|
||||
project_id = _int_param_or_none(request.GET.get('project'))
|
||||
team_id = _int_param_or_none(request.GET.get('team'))
|
||||
|
||||
rows = _build_worker_report_context(status=status, project_id=project_id, team_id=team_id)
|
||||
|
||||
@ -1788,18 +1801,27 @@ def worker_batch_report_pdf(request):
|
||||
from .utils import render_to_pdf
|
||||
|
||||
status = request.GET.get('status') or 'all'
|
||||
project_id = request.GET.get('project') or None
|
||||
team_id = request.GET.get('team') or None
|
||||
project_id = _int_param_or_none(request.GET.get('project'))
|
||||
team_id = _int_param_or_none(request.GET.get('team'))
|
||||
|
||||
rows = _build_worker_report_context(status=status, project_id=project_id, team_id=team_id)
|
||||
|
||||
# Filter names are display-only — a deleted project/team (stale
|
||||
# bookmark) degrades to the unfiltered label instead of crashing.
|
||||
try:
|
||||
project_name = Project.objects.get(id=project_id).name if project_id else 'All Projects'
|
||||
except Project.DoesNotExist:
|
||||
project_name = 'All Projects'
|
||||
try:
|
||||
team_name = Team.objects.get(id=team_id).name if team_id else 'All Teams'
|
||||
except Team.DoesNotExist:
|
||||
team_name = 'All Teams'
|
||||
|
||||
context = {
|
||||
'rows': rows,
|
||||
'status': status,
|
||||
'project_name': (
|
||||
Project.objects.get(id=project_id).name if project_id else 'All Projects'
|
||||
),
|
||||
'team_name': Team.objects.get(id=team_id).name if team_id else 'All Teams',
|
||||
'project_name': project_name,
|
||||
'team_name': team_name,
|
||||
'now': timezone.now(),
|
||||
'total_workers': len(rows),
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user