# 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:
```python
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`  
```python
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:
```python
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`:
```python
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).