penracourses/SECURITY_AUDIT.md

Security Audit Report

Date: 2026-04-30
Scope: All views.py files and generic/decorators.py across the entire project.

---

Issues Fixed

1. Missing @login_required on exam_collection_exams_partial (generic/views.py)

Severity: High
File: generic/views.py
The exam_collection_exams_partial view was publicly accessible with no authentication. It exposed exam names, candidate counts, and author information from any collection to anonymous users.
Fix: Added @login_required decorator.

---

2. Unauthenticated get_*_id popup-helper views (multiple files)

Severity: High
Files: generic/views.py, anatomy/views.py, rapids/views.py, longs/views.py, atlas/views.py

All of the following views were decorated with @csrf_exempt but had no authentication. They allowed unauthenticated users to enumerate internal database records (Examinations, BodyParts, Structures, Abnormalities, Regions) by name via GET requests.

Additionally, all of these views contained a bug: request.accepts("application/json")() calls the boolean result of accepts() as a function, which raises TypeError: 'bool' object is not callable. This made them effectively non-functional (always returning HttpResponse("/")).

File	View
generic/views.py	get_examination_id
anatomy/views.py	get_body_part_id, get_examination_id, get_structure_id
rapids/views.py	get_abnormality_id, get_examination_id, get_region_id
longs/views.py	get_examination_id
atlas/views.py	get_examination_id

Fix:

• Removed @csrf_exempt (GET-only views do not need CSRF exemption).
• Added @login_required.
• Fixed request.accepts("application/json")() → request.accepts("application/json").
• Replaced request.GET["key"] dict access (raises KeyError on missing key) with request.GET.get("key", "").
• Added docstrings documenting scope and functionality.

---

3. Unauthenticated answer_suggestion_submit with CSRF exemption (rad/views.py)

Severity: High
File: rad/views.py (line ~781)

This view accepted anonymous POST requests with no CSRF protection to create new Answer records (marked proposed=True) on any existing question. This allowed anyone on the internet to submit arbitrary proposed answers to anatomy or rapids questions, polluting the moderation queue.

Fix:

• Removed @csrf_exempt.
• Added @login_required.
• Added if request.method != "POST": return ... guard.
• Fixed a minor typo in the response message ("submited" → "submitted").
• Added docstring documenting scope and functionality.

---

4. Debug print() in check_user_in_group decorator (generic/decorators.py)

Severity: Medium
File: generic/decorators.py

The check_user_in_group decorator printed request.user to stdout on every invocation, leaking usernames/email addresses to application logs.
Fix: Removed the print(request.user) statement.

---

5. Debug print() statements leaking sensitive data (multiple files)

Severity: Medium
Files: generic/views.py, atlas/views.py, longs/views.py, sbas/views.py, physics/views.py, shorts/views.py, rcr/views.py, oef/views.py

Numerous active (non-commented) print() calls throughout view code leaked sensitive data to stdout/production logs, including:

• User objects and usernames
• CID numbers and passcodes
• Email address lists being processed
• Exam and queryset data

Fix: All active print() statements replaced with logger.debug() (using the project-standard loguru logger) or removed where they provided no diagnostic value. Commented-out print statements were left in place.

---

6. @csrf_exempt on collection_case_displaysetup (atlas/views.py)

Severity: Low
File: atlas/views.py

The view had @csrf_exempt applied alongside a comment saying "Only if you have CSRF issues; otherwise, keep CSRF protection". The view already has proper authentication via @user_is_collection_author_or_atlas_editor, so the CSRF exemption was unnecessary and weakened security.
Fix: Removed @csrf_exempt.

---

7. Missing @login_required on toggle_share_with_supervisor (generic/views.py)

Severity: Medium
File: generic/views.py

This HTMX view had no authentication check. An anonymous user with a known CidUserExam PK could toggle the share_with_supervisor flag (although the request.user != cid_user_exam.user_user check would prevent the toggle from succeeding, it still leaks whether the record exists).
Fix: Added @login_required and updated docstring.

---

8. Missing @login_required on active_exams in sbas/views.py

Severity: Medium
File: sbas/views.py

The active_exams view had no authentication. It returned a list of all active SBA exams to anonymous users.
Fix: Added @login_required. Updated docstring.

---

9. Unreachable dead code in rcr/views.py

Severity: Low
File: rcr/views.py

In the get_object method of an UpdateView subclass, two lines appeared after a return obj statement:

return obj
print("test")        # unreachable
return super().get_object()  # unreachable

Fix: Removed the unreachable print("test") and return super().get_object() lines.

---

Issues Requiring Clarification (Not Fixed)

A. @csrf_exempt on exam_submit (rad/views.py)

File: rad/views.py
This view accepts POST submissions of exam answers without CSRF protection and without @login_required. It appears to be a legacy mobile/external client endpoint that dispatches to post_exam_answers() on individual exam view classes, which may perform their own CID/passcode authentication.

Risk: Without authentication at this layer, the endpoint could be abused to submit bogus exam answers for real users if a valid CID/passcode combination is known.

Recommendation: Clarify whether this endpoint is still in active use by a mobile client. If so, implement token-based authentication (e.g. Django REST Framework token auth) or require CID/passcode in the request body and validate before dispatching. If not in use, remove it.

---

B. Unauthenticated views in rcr/views.py

File: rcr/views.py
The following views have no authentication decorators:

• radiology_gap — renders a full RCR gap analysis page with all curriculum items
• radiology_results — renders all RCR assessment results

Risk: If these pages are intended to be internal-only (for assessors), they should require authentication. If they are intentionally public (e.g., public-facing curriculum overview), this is acceptable.

Recommendation: Confirm intended visibility. If internal, add @login_required (or a more specific decorator).

---

C. Unauthenticated class-based views in oef/views.py

File: oef/views.py
The following CBVs lack LoginRequiredMixin:

• EntryUpdateView — allows updating OEF entry fields
• EntryTableView — lists all entries
• FormatsCreateView — creates new Formats objects
• FormatsUpdateView — updates Formats objects

The update views in particular are a concern if this is not an intentionally public tool.

Recommendation: Add LoginRequiredMixin (and appropriate permission checks) if this app is not intended to be publicly editable.

---

D. Commented-out @login_required on exam_toggle_flag in physics/views.py

File: physics/views.py
The exam_toggle_flag view has #@login_required commented out. The view falls back to exam.check_user_can_take(cid, passcode, request.user) for access control, suggesting this is intentionally accessible to CID users (non-Django-auth users). However, the intent should be confirmed.

Recommendation: If the CID/passcode check is the intended authentication mechanism, document this clearly. If the view should also support Django-authenticated users, restore the decorator appropriately.

---

E. exam_submit potential AttributeError (rad/views.py)

File: rad/views.py

exam_type, exam_id = request.POST.get("eid").split("/")

If eid is missing from the POST body, request.POST.get("eid") returns None, and .split("/") raises AttributeError. This would return a 500 response rather than a clean error.

Recommendation: Guard with:

eid = request.POST.get("eid", "")
if "/" not in eid:
    return JsonResponse({"success": False, "error": "Invalid eid"})
exam_type, exam_id = eid.split("/", 1)

---

F. Open redirect risk in RedirectMixin and UpdateUserProfileView (generic/views.py, rad/views.py)

File: generic/views.py, rad/views.py
Both RedirectMixin.get_success_url and UpdateUserProfileView.get_success_url use self.request.GET["redirect"] / self.request.GET.get("redirect") as the redirect target without validation. A malicious actor could craft a URL like ?redirect=https://evil.com to redirect authenticated users to a phishing site after a successful form submission.

Recommendation: Validate the redirect URL with Django's url_has_allowed_host_and_scheme:

from django.utils.http import url_has_allowed_host_and_scheme

redirect_url = self.request.GET.get("redirect", "")
if url_has_allowed_host_and_scheme(redirect_url, allowed_hosts={self.request.get_host()}):
    return redirect_url
return super().get_success_url()

---

G. Missing login check on sbas/views.py #@login_required commented-out views

File: sbas/views.py
Two views have @login_required commented out:

• question_view (around line 222) — currently commented out entirely
• question_detail (around line 226) — currently commented out entirely

These are fully commented-out functions; no action needed unless they are re-enabled.

---

Notes on Existing @csrf_exempt Usage

The following @csrf_exempt usages remain after fixes. All are on GET-only admin popup helpers that are now also @login_required. CSRF protection is not relevant for idempotent GET requests, but the decorators are harmless:

All @csrf_exempt instances have been removed from the codebase as part of this audit. The pattern @csrf_exempt on GET-only views is redundant since CSRF protection only applies to state-changing requests (POST/PUT/PATCH/DELETE).