From b8a9ee94a9918512008384c555863663215b202c Mon Sep 17 00:00:00 2001 From: Quinten Steenhuis Date: Mon, 19 Oct 2026 21:04:46 -0400 Subject: [PATCH] Fix credential disclosure and enforce filing request boundaries --- docs/developer-notes/security-hardening.md | 13 +- efile_app/efile/api/auth_views.py | 30 +- efile_app/efile/api/filing_views.py | 14 +- efile_app/efile/api/suffolk_api_views.py | 25 +- efile_app/efile/services/account_profile.py | 6 +- efile_app/efile/services/corrections.py | 25 +- efile_app/efile/services/court_payments.py | 4 + efile_app/efile/services/drafts.py | 2 +- efile_app/efile/services/efsp_payload.py | 67 +++- efile_app/efile/services/existing_cases.py | 11 +- efile_app/efile/services/filings.py | 6 +- efile_app/efile/services/payment_accounts.py | 9 + efile_app/efile/static/js/filing-payload.js | 1 + efile_app/efile/tests/test_corrections.py | 5 +- efile_app/efile/tests/test_disclaimers.py | 7 +- .../tests/test_disclosure_regressions.py | 361 ++++++++++++++++++ efile_app/efile/tests/test_durable_drafts.py | 42 +- efile_app/efile/tests/test_efsp_payload.py | 16 +- efile_app/efile/tests/test_efsp_validation.py | 16 +- .../efile/tests/test_filing_availability.py | 12 + .../efile/tests/test_filing_integrity.py | 6 +- efile_app/efile/tests/tests.py | 4 +- efile_app/efile/utils/efsp_security.py | 50 +++ efile_app/efile/views/session_api.py | 6 +- efile_app/efile/views/submission.py | 4 +- 25 files changed, 641 insertions(+), 101 deletions(-) create mode 100644 efile_app/efile/tests/test_disclosure_regressions.py create mode 100644 efile_app/efile/utils/efsp_security.py diff --git a/docs/developer-notes/security-hardening.md b/docs/developer-notes/security-hardening.md index 67b49e41..dd7f69c2 100644 --- a/docs/developer-notes/security-hardening.md +++ b/docs/developer-notes/security-hardening.md @@ -26,9 +26,20 @@ Rebuild the image so `uv sync --frozen` installs the new lockfile and `collectst ## Follow-up work - Aggregate upload limits and storage quotas, actual file-content validation, durable upload staging, and orphan cleanup. -- Restore CSRF checks on browser session mutation and submission endpoints, with matching JavaScript changes and CSRF-enforcing tests. - Stop unnecessary session writes on polling; test expiry behavior before changing the global session setting. - Measure web workflows and extraction recovery under realistic load, including PostgreSQL contention, upstream rate limits, and difficult PDFs. Unit interleaving tests do not establish production throughput. - Validate and version compiled YAML configurations; add derived-response caching only where profiling shows a benefit. The submission claim and ambiguous-outcome safeguards are unchanged. Extraction retries must not be reused as an automatic court-submission retry mechanism. + +## Confidentiality fixes from issue #345 + +The browser token API returns only the active account's jurisdiction-specific Tyler token, never the EFSP service key. Profile, payment, fee, and filing-history APIs require both a matching Django identity and a court token. Responses must not be cached. Jurisdictions must match installed configurations; case-lookup court codes cannot contain path traversal, URL delimiters, or encoded aliases. These proxy requests do not follow redirects carrying credentials. + +Browser login, logout, session updates, fee quotes, case lookup POSTs, and submissions use Django's CSRF middleware. JSON login also requires the JSON content type and does not echo malformed credentials. Existing browser requests send the CSRF token; bearer-authenticated interview handoff APIs retain their separate authentication boundary. + +Fee quotes and submissions rebuild each document from the current draft's private S3 key and saved filing selections, including its confidentiality code. The proxy receives a fresh one-hour presigned URL. Browser URLs, nested attachments, page counts, and arbitrary document properties are discarded. Missing drafts, unavailable documents, and stale document IDs fail closed. Corrections use the same document binding. Court-code normalization is separate from this mandatory preparation step; forwarding views must call `prepare_efile_payload`. + +Deploy the patch to every exposed environment, then rotate and revoke previously exposed EFSP service keys and review proxy access logs. A source patch cannot revoke a key already obtained. This PR does not rotate keys or change deployed services. Confirm the deployed EFSP and Gotenberg versions and outbound network policies separately; the audit did not establish production metadata theft or read live infrastructure settings. + +Regression coverage uses CSRF-enforcing clients, synthetic accounts, owned and foreign draft documents, and mocked external storage/HTTP. It checks both forwarding boundaries, rejected traversal, nested attachment removal, and successful same-origin login. Bandit alone does not detect these trust-boundary failures. diff --git a/efile_app/efile/api/auth_views.py b/efile_app/efile/api/auth_views.py index 2dc72640..a6bb46fb 100644 --- a/efile_app/efile/api/auth_views.py +++ b/efile_app/efile/api/auth_views.py @@ -9,7 +9,6 @@ from django.conf import settings from django.contrib.auth import authenticate, login from django.http import JsonResponse -from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_http_methods from requests.exceptions import RequestException, Timeout @@ -18,6 +17,7 @@ from efile.services.court_payments import BANK_ACCOUNT_TYPE, bank_payment_policy, policy_message from efile.services.current_drafts import get_current_draft from efile.services.payment_accounts import is_active +from efile.utils.efsp_security import require_efsp_login, validated_jurisdiction from efile.utils.jurisdiction_stuff import get_jurisdiction_from_request from .base import APIResponseMixin @@ -30,15 +30,18 @@ class AuthAPIViews(APIResponseMixin): @staticmethod @require_http_methods(["POST"]) - @csrf_exempt def user_login(request): """Handle user login""" + if request.content_type != "application/json": + return AuthAPIViews.error_response("Expected application/json", 415) try: data = json.loads(request.body) + if not isinstance(data, dict): + return AuthAPIViews.error_response("Expected a JSON object", 400) username = data.get("username") password = data.get("password") - jurisdiction = data.get("jurisdiction") + jurisdiction = validated_jurisdiction(data.get("jurisdiction")) if not username or not password or not jurisdiction: return AuthAPIViews.error_response("Username, password, and jurisdiction required") @@ -64,14 +67,13 @@ def user_login(request): else: return AuthAPIViews.error_response("Invalid credentials", 401) - except json.JSONDecodeError as e: - return AuthAPIViews.error_response(f"Invalid JSON data: {str(e)}, {request.body}") + except (ValueError, UnicodeDecodeError): + return AuthAPIViews.error_response("Invalid login data", 400) except Exception as e: return AuthAPIViews.error_response(f"Error: {str(e)}") @staticmethod @require_http_methods(["POST"]) - @csrf_exempt def user_logout(request): """Handle user logout""" from efile.authentication import SuffolkEFileBackend @@ -84,6 +86,7 @@ def user_logout(request): @staticmethod @require_http_methods(["GET"]) + @require_efsp_login def user_profile(request): """Get current user profile from external Suffolk eFile API""" try: @@ -154,22 +157,20 @@ def external_profile(request): @staticmethod @require_http_methods(["GET"]) + @require_efsp_login def tyler_token(request): - """Get Tyler token and API key for external form submissions""" + """Return only the signed-in user's token for the payment form.""" try: # Get jurisdiction and Tyler token dynamically jurisdiction = get_jurisdiction_from_request(request) tyler_token = get_tyler_token(request, jurisdiction) - api_key = getattr(settings, "SUFFOLK_EFILE_API_KEY", None) - - return AuthAPIViews.success_response( - {"tyler_token": tyler_token, "api_key": api_key, "state": jurisdiction} - ) + return AuthAPIViews.success_response({"tyler_token": tyler_token, "state": jurisdiction}) except Exception as e: return AuthAPIViews.error_response(f"Error: {str(e)}") @staticmethod @require_http_methods(["GET"]) + @require_efsp_login def payment_accounts(request): """Get payment accounts from Suffolk eFile API with proper authentication""" try: @@ -196,7 +197,7 @@ def payment_accounts(request): url = f"{settings.EFSP_URL}/jurisdictions/{jurisdiction}/payments/payment-accounts/" logger.debug("GET %s header keys=%s", url, list(headers.keys())) - api_response = requests.get(url, headers=headers, timeout=10) + api_response = requests.get(url, headers=headers, timeout=10, allow_redirects=False) logger.debug( "Payment accounts response: status=%s content_type=%s", api_response.status_code, @@ -251,6 +252,7 @@ def payment_accounts(request): @staticmethod @require_http_methods(["GET"]) + @require_efsp_login def payment_account_types(request): """List the court's payment account types (e.g. "CC", "WV") with names. @@ -273,7 +275,7 @@ def payment_account_types(request): url = f"{settings.EFSP_URL}/jurisdictions/{jurisdiction}/payments/types" logger.debug("GET %s header keys=%s", url, list(headers.keys())) - api_response = requests.get(url, headers=headers, timeout=10) + api_response = requests.get(url, headers=headers, timeout=10, allow_redirects=False) if api_response.status_code == 200: return AuthAPIViews.success_response(api_response.json()) diff --git a/efile_app/efile/api/filing_views.py b/efile_app/efile/api/filing_views.py index 9ffa9810..fdf2fb99 100644 --- a/efile_app/efile/api/filing_views.py +++ b/efile_app/efile/api/filing_views.py @@ -8,9 +8,9 @@ import requests from django.conf import settings from django.http import JsonResponse -from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_http_methods +from efile.utils.efsp_security import require_efsp_login, validated_code, validated_jurisdiction from efile.utils.jurisdiction_stuff import get_jurisdiction_from_request from ..services.court_payments import payment_error, validate_filing_payment @@ -51,6 +51,7 @@ def list_filing_data(request, jurisdiction, *, start_date=None): accidentally validate a browser-supplied case ID instead. """ + jurisdiction = validated_jurisdiction(jurisdiction) api_url = f"{settings.EFSP_URL}/jurisdictions/{jurisdiction}/filingreview/courts/0/filings" headers = get_headers() tyler_token = get_tyler_token(request, jurisdiction) @@ -64,6 +65,7 @@ def list_filing_data(request, jurisdiction, *, start_date=None): params={"start_date": start_date or None, "before_date": None}, headers=headers, timeout=30, + allow_redirects=False, ) logger.debug( "Get filings response: status=%s content_type=%s", @@ -97,6 +99,7 @@ class FilingAPIViews(APIResponseMixin): @staticmethod @require_http_methods(["GET"]) + @require_efsp_login def get_filings(request): try: # Get all filings in all courts between now and a specified time @@ -148,7 +151,7 @@ def convert_filing_data(filing): @staticmethod @require_http_methods(["POST"]) - @csrf_exempt + @require_efsp_login(session_jurisdiction=True) def payment_fees(request): try: logger.info("Test in payment fees!") @@ -157,7 +160,7 @@ def payment_fees(request): if not efile_data: return JsonResponse({"success": False, "error": "No efile data provided in request"}, status=400) - jurisdiction_id = request.session.get("jurisdiction") + jurisdiction_id = validated_jurisdiction(request.session.get("jurisdiction")) auth_tokens = request.session.get("auth_tokens", {}) case_data = get_case_data(request, jurisdiction_id) court_id = case_data.get("court", "") @@ -165,6 +168,7 @@ def payment_fees(request): draft = get_current_draft(request, jurisdiction=jurisdiction_id, resume_latest=False) if draft is not None: court_id = draft.court_code + court_id = validated_code(court_id) account_id = str(data.get("payment_account_id") or (draft.selected_payment_account_id if draft else "")) try: validate_filing_payment( @@ -213,13 +217,13 @@ def payment_fees(request): priced_current_draft = draft is not None and bool(inputs_token) and fee_inputs_token(draft) == inputs_token logger.info(f"Making request!: {url}") - response = requests.post(url, json=efile_data, headers=headers, timeout=60) + response = requests.post(url, json=efile_data, headers=headers, timeout=60, allow_redirects=False) logger.info(f"Made request: {response.status_code}") if response.status_code == 200 or response.status_code == 201: response_data = response.json() - logger.info(f"Sending back: {response_data}") + logger.debug("Fee calculation succeeded") # Kept on the draft with what it was priced on, so Review shows # it only while it still describes the filing. A quote for a # version of the filing that has since changed is not kept. diff --git a/efile_app/efile/api/suffolk_api_views.py b/efile_app/efile/api/suffolk_api_views.py index 7c66e4be..1f1e61d2 100644 --- a/efile_app/efile/api/suffolk_api_views.py +++ b/efile_app/efile/api/suffolk_api_views.py @@ -8,12 +8,12 @@ import requests from django.conf import settings from django.http import JsonResponse -from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_http_methods from requests.exceptions import RequestException from efile.utils.case_data_utils import get_case_data, update_case_data -from efile.utils.jurisdiction_stuff import get_jurisdiction_from_request, get_jurisdiction_token +from efile.utils.efsp_security import validated_code, validated_jurisdiction +from efile.utils.jurisdiction_stuff import get_jurisdiction_from_request, get_jurisdiction_token, has_jurisdiction_login from efile.utils.proxy_connection import get_headers logger = logging.getLogger(__name__) @@ -34,7 +34,6 @@ def set_access_control_headers(response): return response -@csrf_exempt @require_http_methods(["GET", "POST", "OPTIONS"]) def lookup_case(request): """ @@ -73,6 +72,10 @@ def lookup_case(request): state = jurisdiction.lower() else: state = get_jurisdiction_from_request(request) + state = validated_jurisdiction(state) + court = validated_code(court) + if not has_jurisdiction_login(request, state): + return JsonResponse({"success": False, "error": "Sign in to this jurisdiction first."}, status=401) api_url = f"{settings.EFSP_URL}/jurisdictions/{state}/cases/courts/{court}/cases" # Add query parameter for docket number @@ -95,7 +98,7 @@ def lookup_case(request): ) # Make the API request - using GET with query parameters - response = requests.get(api_url, params=params, headers=headers, timeout=30) + response = requests.get(api_url, params=params, headers=headers, timeout=30, allow_redirects=False) response.raise_for_status() api_data = response.json() @@ -119,8 +122,8 @@ def lookup_case(request): # Extract case category if "caseCategoryText" in case_value and "value" in case_value["caseCategoryText"]: case_info["caseCategoryCode"] = case_value["caseCategoryText"]["value"] - api_url = f"{settings.EFSP_URL}/jurisdictions/{state}/codes/courts/{court}/categories/{case_info['caseCategoryCode']}" - category_resp = requests.get(api_url, headers=headers, timeout=30) + api_url = f"{settings.EFSP_URL}/jurisdictions/{state}/codes/courts/{court}/categories/{validated_code(case_info['caseCategoryCode'])}" + category_resp = requests.get(api_url, headers=headers, timeout=30, allow_redirects=False) if category_resp.ok: data = category_resp.json() case_info["caseCategoryName"] = data["name"] @@ -142,8 +145,10 @@ def lookup_case(request): if "value" in item and "caseTypeText" in item["value"]: if "value" in item["value"]["caseTypeText"]: case_info["caseTypeCode"] = item["value"]["caseTypeText"]["value"] - api_url = f"{settings.EFSP_URL}/jurisdictions/{state}/codes/courts/{court}/case_types/{case_info['caseTypeCode']}" - case_resp = requests.get(api_url, headers=headers, timeout=30) + api_url = f"{settings.EFSP_URL}/jurisdictions/{state}/codes/courts/{court}/case_types/{validated_code(case_info['caseTypeCode'])}" + case_resp = requests.get( + api_url, headers=headers, timeout=30, allow_redirects=False + ) if case_resp.ok: data = case_resp.json() case_info["caseTypeName"] = data["name"] @@ -174,9 +179,9 @@ def lookup_case(request): return set_access_control_headers(response) - except json.JSONDecodeError: + except (ValueError, UnicodeDecodeError): # This only applies to POST requests - response = JsonResponse({"success": False, "error": "Invalid JSON in request body"}, status=400) + response = JsonResponse({"success": False, "error": "Invalid jurisdiction, court, or request data"}, status=400) return set_access_control_headers(response) except RequestException as e: diff --git a/efile_app/efile/services/account_profile.py b/efile_app/efile/services/account_profile.py index 3947478b..9ecee1c7 100644 --- a/efile_app/efile/services/account_profile.py +++ b/efile_app/efile/services/account_profile.py @@ -78,8 +78,10 @@ def fetch_account_profile(request, jurisdiction, timeout=10): headers = _headers(request, jurisdiction) base = f"{settings.EFSP_URL}/jurisdictions/{jurisdiction}" try: - firm_response = requests.get(f"{base}/firmattorneyservice/firm", headers=headers, timeout=timeout) - self_response = requests.get(f"{base}/adminusers/user", headers=headers, timeout=timeout) + firm_response = requests.get( + f"{base}/firmattorneyservice/firm", headers=headers, timeout=timeout, allow_redirects=False + ) + self_response = requests.get(f"{base}/adminusers/user", headers=headers, timeout=timeout, allow_redirects=False) except RequestException: logger.exception("Could not reach the e-filing service for the account profile") return None diff --git a/efile_app/efile/services/corrections.py b/efile_app/efile/services/corrections.py index 6407b10d..583a794e 100644 --- a/efile_app/efile/services/corrections.py +++ b/efile_app/efile/services/corrections.py @@ -376,8 +376,7 @@ def correction_readiness(draft): def refresh_correction_documents(payload, draft): """Use the reviewed durable bytes and metadata, never a stale browser URL.""" - from efile.services.efsp_payload import PayloadValidationError - from efile.utils.s3_upload_handler import S3UploadHandler + from efile.services.efsp_payload import bind_filing_documents payload.update( efile_case_category=draft.case_category_code, @@ -394,24 +393,4 @@ def refresh_correction_documents(payload, draft): payload["docket_number"] = draft.docket_number else: payload.pop("docket_number", None) - documents = list(draft.documents.order_by("role", "sort_order", "pk")) - bundles = payload.get("al_court_bundle") - if not isinstance(bundles, list) or len(bundles) != len(documents): - raise PayloadValidationError("The documents changed. Reload Review before submitting.") - handler = S3UploadHandler() - for bundle, document in zip(bundles, documents, strict=True): - if not isinstance(bundle, dict) or not document.s3_key: - raise PayloadValidationError("Replace the unavailable PDF before submitting.") - try: - url = handler.get_public_url(document.s3_key) - except (BotoCoreError, ClientError, RuntimeError) as error: - raise PayloadValidationError("Document storage is unavailable. Retry before submitting.") from error - bundle.update( - data_url=url, - filename=document.name, - filing_type=document.filing_type_code, - document_type=document.document_type_code, - filing_component=document.filing_component_code, - optional_services=document.requested_optional_services, - courtesy_copies=[document.courtesy_copy_email] if document.courtesy_copy_email else [], - ) + bind_filing_documents(payload, draft) diff --git a/efile_app/efile/services/court_payments.py b/efile_app/efile/services/court_payments.py index 197abb51..cfae3d08 100644 --- a/efile_app/efile/services/court_payments.py +++ b/efile_app/efile/services/court_payments.py @@ -12,6 +12,7 @@ from efile.services.draft_urls import draft_url from efile.services.payment_accounts import account_headers, payment_accounts +from efile.utils.efsp_security import validated_code, validated_jurisdiction BANK_ACCOUNT_TYPE = "BankAccount" RESTRICTED_MESSAGE = ( @@ -25,10 +26,13 @@ def bank_payment_policy(jurisdiction, court): if not court: return "unknown" try: + jurisdiction = validated_jurisdiction(jurisdiction) + court = validated_code(court) response = requests.get( f"{settings.EFSP_URL}/jurisdictions/{quote(jurisdiction, safe='')}/codes/courts/{quote(court, safe='')}/codes", headers=account_headers(jurisdiction, ""), timeout=10, + allow_redirects=False, ) response.raise_for_status() data = response.json() diff --git a/efile_app/efile/services/drafts.py b/efile_app/efile/services/drafts.py index 38da2398..30a02e5a 100644 --- a/efile_app/efile/services/drafts.py +++ b/efile_app/efile/services/drafts.py @@ -593,7 +593,7 @@ def write_upload_data( def _document_file(doc: FilingDocument, urls: dict[int, str] | None = None) -> dict[str, Any]: - file_obj: dict[str, Any] = {} + file_obj: dict[str, Any] = {"document_id": doc.pk} _put(file_obj, "name", doc.name) _put(file_obj, "url", (urls or {}).get(doc.pk, doc.public_url)) _put(file_obj, "s3_key", doc.s3_key) diff --git a/efile_app/efile/services/efsp_payload.py b/efile_app/efile/services/efsp_payload.py index 65ce3248..f48a23d6 100644 --- a/efile_app/efile/services/efsp_payload.py +++ b/efile_app/efile/services/efsp_payload.py @@ -10,10 +10,13 @@ import time import requests +from botocore.exceptions import BotoCoreError, ClientError from django.conf import settings from django.core.exceptions import ImproperlyConfigured from efile.services.efsp_validation import party_validation, validate_party +from efile.utils.efsp_security import validated_code, validated_jurisdiction +from efile.utils.s3_upload_handler import S3UploadHandler logger = logging.getLogger(__name__) @@ -132,11 +135,73 @@ def prepare_efile_payload(efile_data, jurisdiction_id, court_id, *, draft=None): """ from efile.services.existing_cases import reconcile_case_parties - if draft is not None and draft.correction_of_id: + try: + validated_jurisdiction(jurisdiction_id) + validated_code(court_id) + except ValueError as error: + raise PayloadValidationError(str(error)) from error + if draft is None or draft.jurisdiction != jurisdiction_id or draft.court_code != court_id: + raise PayloadValidationError("Open your filing and review it before continuing.") + if draft.correction_of_id: from efile.services.corrections import refresh_correction_documents refresh_correction_documents(efile_data, draft) + else: + bind_filing_documents(efile_data, draft) reconcile_case_parties(efile_data, draft, jurisdiction_id, court_id) + return normalize_efile_payload(efile_data, jurisdiction_id, court_id) + + +def bind_filing_documents(payload, draft): + """Rebuild every document from this draft's stored bytes and selections. + + Discard nested ``elements`` too: the proxy prefers those attachments over a + parent's data_url. Replacing only the parent URL would leave a fetch bypass. + """ + documents = list(draft.documents.order_by("role", "sort_order", "pk")) + bundles = payload.get("al_court_bundle") + if not documents or not isinstance(bundles, list) or len(bundles) != len(documents): + raise PayloadValidationError("The documents changed. Reload Review before continuing.") + handler = S3UploadHandler() + rebuilt = [] + for bundle, document in zip(bundles, documents, strict=True): + if not isinstance(bundle, dict) or not document.s3_key: + raise PayloadValidationError("Replace the unavailable PDF before continuing.") + # Older open tabs have no document_id. Their bundle order is the same + # lead/supporting order; all fetch and filing metadata is still rebuilt. + if "document_id" in bundle and str(bundle["document_id"]) != str(document.pk): + raise PayloadValidationError("The documents changed. Reload Review before continuing.") + try: + url = handler.get_public_url(document.s3_key, expiration=3600) + except (BotoCoreError, ClientError, RuntimeError) as error: + raise PayloadValidationError("Document storage is unavailable. Retry before continuing.") from error + rebuilt.append( + { + "proxy_enabled": True, + "data_url": url, + "filename": document.name, + "filing_type": document.filing_type_code, + "document_type": document.document_type_code, + "filing_component": document.filing_component_code, + "filing_description": document.filing_type_name or document.name, + "document_description": document.document_type_name, + "optional_services": list(document.requested_optional_services or []), + "courtesy_copies": [document.courtesy_copy_email] if document.courtesy_copy_email else [], + "preliminary_copies": [], + "filing_parties": [f"users[{index}]" for index, _ in enumerate(payload.get("users", []))], + "filing_action": "efile", + "due_date": None, + "reference_number": "", + "filing_attorney": "", + "filing_comment": "", + "tyler_merge_attachments": False, + } + ) + payload["al_court_bundle"] = rebuilt + + +def normalize_efile_payload(efile_data, jurisdiction_id, court_id): + """Normalize already bound documents using the court's code lists.""" lookups = _EfspLookups() _clean_case_identifiers(efile_data) validate_party_formats(efile_data, jurisdiction_id, court_id) diff --git a/efile_app/efile/services/existing_cases.py b/efile_app/efile/services/existing_cases.py index d4999caf..44666570 100644 --- a/efile_app/efile/services/existing_cases.py +++ b/efile_app/efile/services/existing_cases.py @@ -19,6 +19,7 @@ from efile.party_sides import side_for_party_type_name from efile.services.drafts import ACTIVE_DRAFT_STATUSES from efile.services.fee_quotes import FEE_QUOTE_FIELDS, fee_inputs_token, invalidate_fee_quote +from efile.utils.efsp_security import validated_code, validated_jurisdiction from efile.utils.proxy_connection import get_headers from efile.workflow import ExistingCase @@ -275,6 +276,12 @@ def normalize_case(raw, *, court, tracking_id, party_types, docket_number=""): def fetch_case(draft, token): if not token: raise CaseImportError("Sign in again to load your court case.") + try: + validated_jurisdiction(draft.jurisdiction) + validated_code(draft.court_code) + validated_code(draft.previous_case_id) + except ValueError as error: + raise CaseImportError("Choose a valid court and case before loading parties.") from error url = ( f"{settings.EFSP_URL}/jurisdictions/{quote(draft.jurisdiction, safe='')}/cases/" f"courts/{quote(draft.court_code, safe='')}/cases/{quote(draft.previous_case_id, safe='')}" @@ -287,10 +294,10 @@ def fetch_case(draft, token): headers[f"tyler-token-{draft.jurisdiction}"] = token # The role list does not depend on the case, so fetch both at once. pool = ThreadPoolExecutor(max_workers=1) - pending_codes = pool.submit(requests.get, codes_url, headers=headers, timeout=15) + pending_codes = pool.submit(requests.get, codes_url, headers=headers, timeout=15, allow_redirects=False) pool.shutdown(wait=False) try: - response = requests.get(url, headers=headers, timeout=30) + response = requests.get(url, headers=headers, timeout=30, allow_redirects=False) response.raise_for_status() raw = response.json() except (requests.RequestException, ValueError) as error: diff --git a/efile_app/efile/services/filings.py b/efile_app/efile/services/filings.py index f2045560..a393c8dd 100644 --- a/efile_app/efile/services/filings.py +++ b/efile_app/efile/services/filings.py @@ -29,6 +29,7 @@ from efile.models import ArchivedCase from efile.services.filing_timing import add_tracker_timing from efile.utils.config_loader import config_loader +from efile.utils.efsp_security import validated_code, validated_jurisdiction from efile.utils.proxy_connection import get_headers logger = logging.getLogger(__name__) @@ -381,6 +382,9 @@ def fetch_filing_detail(request, jurisdiction: str, court_code: str, filing_id: detail endpoint rejects the envelope number with a 422. """ + jurisdiction = validated_jurisdiction(jurisdiction) + court_code = validated_code(court_code) + filing_id = validated_code(filing_id) url = f"{settings.EFSP_URL}/jurisdictions/{jurisdiction}/filingreview/courts/{court_code}/filings/{filing_id}" headers = get_headers() tyler_token = get_tyler_token(request, jurisdiction) @@ -390,7 +394,7 @@ def fetch_filing_detail(request, jurisdiction: str, court_code: str, filing_id: logger.info("No Tyler token for jurisdiction %s in filing-detail request", jurisdiction) return None - response = requests.get(url, headers=headers, timeout=30) + response = requests.get(url, headers=headers, timeout=30, allow_redirects=False) if not response.ok: logger.info( "Filing detail request failed: status=%s court=%s jurisdiction=%s", diff --git a/efile_app/efile/services/payment_accounts.py b/efile_app/efile/services/payment_accounts.py index 8370e81c..02d773e0 100644 --- a/efile_app/efile/services/payment_accounts.py +++ b/efile_app/efile/services/payment_accounts.py @@ -7,6 +7,8 @@ from django.contrib.auth import get_user_model from django.db import transaction +from efile.utils.efsp_security import validated_code, validated_jurisdiction + MANAGED_WAIVER_NAME = "LITEfile managed waiver account" @@ -26,10 +28,12 @@ def is_active(account): def payment_accounts(jurisdiction, token): + jurisdiction = validated_jurisdiction(jurisdiction) response = requests.get( f"{settings.EFSP_URL}/jurisdictions/{jurisdiction}/payments/payment-accounts/", headers=account_headers(jurisdiction, token), timeout=10, + allow_redirects=False, ) response.raise_for_status() accounts = response.json() @@ -43,6 +47,7 @@ def payment_accounts(jurisdiction, token): def ensure_waiver_account(user, jurisdiction, token, preferred_id=""): + jurisdiction = validated_jurisdiction(jurisdiction) # Serialize requests by this user, including requests from other drafts/tabs. # Always re-read Tyler before creating; failed reads must never mean no account. with transaction.atomic(): @@ -55,6 +60,7 @@ def ensure_waiver_account(user, jurisdiction, token, preferred_id=""): data=MANAGED_WAIVER_NAME, headers={**account_headers(jurisdiction, token), "Content-Type": "text/plain"}, timeout=20, + allow_redirects=False, ) response.raise_for_status() try: @@ -72,6 +78,8 @@ def ensure_waiver_account(user, jurisdiction, token, preferred_id=""): def remove_payment_account(jurisdiction, token, account_id): """Remove an account only after finding it in this authenticated account list.""" + jurisdiction = validated_jurisdiction(jurisdiction) + account_id = validated_code(account_id) account = next((a for a in payment_accounts(jurisdiction, token) if str(a["paymentAccountID"]) == account_id), None) if account is None or account.get("paymentAccountTypeCode") == "WV": raise ValueError("Choose a payment account from your account list.") @@ -79,5 +87,6 @@ def remove_payment_account(jurisdiction, token, account_id): f"{settings.EFSP_URL}/jurisdictions/{jurisdiction}/payments/payment-accounts/{quote(account_id, safe='')}", headers=account_headers(jurisdiction, token), timeout=20, + allow_redirects=False, ) response.raise_for_status() diff --git a/efile_app/efile/static/js/filing-payload.js b/efile_app/efile/static/js/filing-payload.js index e54b7564..912eaf65 100644 --- a/efile_app/efile/static/js/filing-payload.js +++ b/efile_app/efile/static/js/filing-payload.js @@ -391,6 +391,7 @@ const FilingPayload = { createDocumentBundle(doc, filingType, documentType, filingComponent, users, description, docDescription, cc_email, optionalServices = []) { const courtesy_copies = cc_email ? [cc_email] : []; return { + document_id: doc.document_id, proxy_enabled: true, filing_type: filingType, optional_services: optionalServices, diff --git a/efile_app/efile/tests/test_corrections.py b/efile_app/efile/tests/test_corrections.py index 00ba1c74..0e6c6eab 100644 --- a/efile_app/efile/tests/test_corrections.py +++ b/efile_app/efile/tests/test_corrections.py @@ -401,7 +401,7 @@ def test_native_copy_renews_urls_from_owned_keys(original, monkeypatch): monkeypatch.setattr("efile.utils.s3_upload_handler.S3UploadHandler._ensure_initialized", lambda self: True) calls = [] - def renewed(self, key): + def renewed(self, key, expiration=604800): calls.append(key) return "https://example.invalid/renewed" @@ -460,7 +460,8 @@ def test_availability_checks_the_refreshed_correction_payload(client, original, monkeypatch.setattr("efile.views.session_api.get_case_data", lambda *args: {"court": revision.court_code}) monkeypatch.setattr("efile.views.session_api.get_upload_data", lambda *args: {"files": {"lead": {"name": "x"}}}) monkeypatch.setattr( - "efile.utils.s3_upload_handler.S3UploadHandler.get_public_url", lambda *args: "https://example.invalid/new" + "efile.utils.s3_upload_handler.S3UploadHandler.get_public_url", + lambda *args, **kwargs: "https://example.invalid/new", ) monkeypatch.setattr( "efile.views.session_api.prepare_efile_payload", diff --git a/efile_app/efile/tests/test_disclaimers.py b/efile_app/efile/tests/test_disclaimers.py index 2109ffce..78f35d41 100644 --- a/efile_app/efile/tests/test_disclaimers.py +++ b/efile_app/efile/tests/test_disclaimers.py @@ -215,7 +215,12 @@ def test_disclaimer_spacing_is_not_controlled_by_empty_court_paragraphs(): @pytest.mark.django_db -def test_submission_without_draft_does_not_reach_tyler(client, monkeypatch): +def test_submission_without_draft_does_not_reach_tyler(client, monkeypatch, django_user_model): + user = django_user_model.objects.create_user(username="no-draft", tyler_jurisdiction="illinois") + client.force_login(user) + session = client.session + session.update({"jurisdiction": "illinois", "auth_tokens": {"TYLER-TOKEN-ILLINOIS": "test-token"}}) + session.save() upstream = Mock() monkeypatch.setattr("efile.views.submission.forward_final_filing", upstream) response = client.post("/api/submit-final-filing/", {"confirm_submission": True}, content_type="application/json") diff --git a/efile_app/efile/tests/test_disclosure_regressions.py b/efile_app/efile/tests/test_disclosure_regressions.py new file mode 100644 index 00000000..cfeae581 --- /dev/null +++ b/efile_app/efile/tests/test_disclosure_regressions.py @@ -0,0 +1,361 @@ +"""Confidentiality regressions use synthetic identities and no external services.""" + +import json +from unittest.mock import Mock, patch + +import pytest +from django.test import Client, RequestFactory + +from efile.models import FilingDocument, FilingDraft +from efile.services.court_payments import bank_payment_policy +from efile.services.efsp_payload import PayloadValidationError, prepare_efile_payload +from efile.services.existing_cases import CaseImportError, fetch_case +from efile.services.filings import fetch_filing_detail +from efile.services.payment_accounts import payment_accounts +from efile.views.session_api import forward_final_filing + +pytestmark = pytest.mark.django_db + + +@pytest.fixture +def signed_in(django_user_model): + client = Client(enforce_csrf_checks=True) + user = django_user_model.objects.create_user(username="filer", tyler_jurisdiction="illinois") + client.force_login(user) + draft = FilingDraft.objects.create(user=user, jurisdiction="illinois", court_code="cook:law1") + session = client.session + session.update( + { + "jurisdiction": "illinois", + "filing_draft_id": draft.pk, + "auth_tokens": {"TYLER-TOKEN-ILLINOIS": "dummy-user-token"}, + } + ) + session.save() + client.get("/jurisdiction/illinois/login/") + return client, draft + + +@pytest.fixture(autouse=True) +def no_external_requests(monkeypatch): + def blocked(*args, **kwargs): + pytest.fail("Unexpected external request") + + monkeypatch.setattr("requests.sessions.Session.send", blocked) + + +@pytest.mark.parametrize("authenticated", [False, True]) +def test_token_response_never_contains_service_key(signed_in, settings, authenticated): + settings.SUFFOLK_EFILE_API_KEY = "private-service-key-marker" + client = signed_in[0] if authenticated else Client(enforce_csrf_checks=True) + response = client.get("/api/auth/tyler-token/?jurisdiction=illinois") + assert response.status_code == (200 if authenticated else 401) + assert settings.SUFFOLK_EFILE_API_KEY.encode() not in response.content + assert "no-store" in response.headers["Cache-Control"] + if authenticated: + assert response.json()["data"] == {"tyler_token": "dummy-user-token", "state": "illinois"} + + +@pytest.mark.parametrize("state", ["vermont", "massachusetts"]) +def test_token_requires_active_identity_for_requested_state(signed_in, state): + client, _ = signed_in + session = client.session + session["auth_tokens"][f"TYLER-TOKEN-{state.upper()}"] = "old-other-state-token" + session.save() + assert client.get("/api/auth/tyler-token/", {"jurisdiction": state}).status_code == 401 + + +def test_token_requires_unexpired_court_login(signed_in): + client, _ = signed_in + session = client.session + session["auth_tokens"] = {} + session.save() + assert client.get("/api/auth/tyler-token/").status_code == 401 + + +@pytest.mark.parametrize( + "path", + [ + "/api/auth/login/", + "/api/auth/logout/", + "/api/save-case-data/", + "/api/save-party-type/", + "/api/fetch-party-type/", + "/api/clear-session/", + "/api/payment-fees/", + "/api/submit-final-filing/", + "/api/suffolk/lookup-case/", + ], +) +def test_cross_origin_browser_mutations_require_csrf(signed_in, path): + client, draft = signed_in + original_session = dict(client.session) + body = ( + '{"username":"attacker@example.invalid","password":"attacker-password","jurisdiction":"illinois","pad":"="}\r\n' + ) + with patch("efile.api.auth_views.authenticate") as authenticate: + response = client.post( + path, body, content_type="text/plain", HTTP_ORIGIN="https://attacker.invalid", secure=True + ) + assert response.status_code == 403 + authenticate.assert_not_called() + assert dict(client.session) == original_session + draft.refresh_from_db() + assert draft.case_title == "" + + +def test_anonymous_login_csrf_cannot_create_attacker_session(): + client = Client(enforce_csrf_checks=True) + with patch("efile.api.auth_views.authenticate") as authenticate: + response = client.post( + "/api/auth/login/", + '{"username":"attacker","password":"test","jurisdiction":"illinois","pad":"="}\r\n', + content_type="text/plain", + HTTP_ORIGIN="https://attacker.invalid", + secure=True, + ) + assert response.status_code == 403 + authenticate.assert_not_called() + assert "_auth_user_id" not in client.session + + +def test_same_origin_json_login_still_works(): + client = Client(enforce_csrf_checks=True) + client.get("/jurisdiction/illinois/login/") + tokens = {"TYLER-TOKEN-ILLINOIS": "dummy-token", "TYLER-ID-ILLINOIS": "dummy-id"} + with patch("efile.authentication.auth_with_tyler_api", return_value={"tokens": tokens}): + response = client.post( + "/api/auth/login/", + {"username": "filer@example.invalid", "password": "dummy-password", "jurisdiction": "illinois"}, + content_type="application/json", + HTTP_X_CSRFTOKEN=client.cookies["csrftoken"].value, + ) + assert response.status_code == 200 + assert response.json()["success"] + assert client.session["auth_tokens"] == tokens + + +@pytest.mark.parametrize( + "bad", ["../../api_user_settings/serverid?", "%2e%2e", "illinois/../vermont", "illinois#", "bogus"] +) +def test_proxy_jurisdiction_injection_is_rejected_before_network(signed_in, bad): + client, _ = signed_in + for endpoint in ["payment-account-types", "payment-accounts", "auth/profile", "auth/tyler-token", "filings"]: + assert client.get(f"/api/{endpoint}/", {"jurisdiction": bad}).status_code == 400 + + +@pytest.mark.parametrize( + "court", ["../../../api_user_settings/serverid?", "..", ".", "%2e%2e", "court?x=1", "court#", "court\\x"] +) +def test_case_lookup_rejects_path_injection(signed_in, court): + client, _ = signed_in + response = client.get( + "/api/suffolk/lookup-case/", {"jurisdiction": "illinois", "court": court, "caseNumber": "123"} + ) + assert response.status_code == 400 + + +def test_payment_types_still_work_without_following_redirects(signed_in): + client, _ = signed_in + with patch( + "efile.api.auth_views.requests.get", return_value=Mock(status_code=200, json=lambda: [{"code": "CC"}]) + ) as get: + response = client.get("/api/payment-account-types/") + assert response.status_code == 200 + assert get.call_args.kwargs["allow_redirects"] is False + assert "/jurisdictions/illinois/payments/types" in get.call_args.args[0] + + +def malicious_payload(): + return { + "users": [], + "al_court_bundle": [ + { + "proxy_enabled": True, + "filing_type": "forged", + "document_type": "public", + "data_url": "http://127.0.0.1/internal", + "page_count": 1, + "elements": [{"proxy_enabled": True, "data_url": "http://169.254.169.254/latest/meta-data"}], + } + ], + } + + +@pytest.mark.parametrize("boundary", ["fees", "submission"]) +def test_both_forwarding_boundaries_use_only_owned_document_bytes(signed_in, boundary, monkeypatch): + client, draft = signed_in + document = FilingDocument.objects.create( + draft=draft, + role="lead", + name="private.pdf", + s3_key="owned/private.pdf", + filing_type_code="petition", + document_type_code="confidential", + filing_component_code="lead", + ) + monkeypatch.setattr("efile.services.efsp_payload._EfspLookups.get", lambda *args: []) + payload = malicious_payload() + with patch("efile.services.efsp_payload.S3UploadHandler") as storage, patch("requests.post") as post: + storage.return_value.get_public_url.return_value = "https://storage.invalid/owned.pdf?signature=dummy" + post.return_value = Mock(status_code=200, json=lambda: {"feesCalculationAmount": {"value": "0"}}) + if boundary == "fees": + response = client.post( + "/api/payment-fees/", + {"efile_data": payload}, + content_type="application/json", + HTTP_X_CSRFTOKEN=client.cookies["csrftoken"].value, + ) + else: + request = RequestFactory().post("/") + request.user, request.session = draft.user, client.session + response = forward_final_filing(request, {"efile_data": payload}) + assert response.status_code == 200, response.content + storage.return_value.get_public_url.assert_called_once_with(document.s3_key, expiration=3600) + forwarded = post.call_args.kwargs["json"] + bundle = forwarded["al_court_bundle"][0] + assert bundle["data_url"] == "https://storage.invalid/owned.pdf?signature=dummy" + assert bundle["document_type"] == "confidential" + assert bundle["filing_type"] == "petition" + assert "elements" not in bundle and "page_count" not in bundle + assert "127.0.0.1" not in json.dumps(forwarded) + assert "169.254.169.254" not in json.dumps(forwarded) + assert post.call_args.kwargs["allow_redirects"] is False + + +@pytest.mark.parametrize( + "problem", + [ + "no-draft", + "empty-draft", + "foreign-document", + "missing-key", + "wrong-jurisdiction", + "wrong-court", + "invalid-court", + ], +) +def test_document_binding_fails_closed(signed_in, problem): + _, draft = signed_in + payload = malicious_payload() + if problem not in {"no-draft", "empty-draft"}: + FilingDocument.objects.create( + draft=draft, role="lead", name="private.pdf", s3_key="" if problem == "missing-key" else "owned/private.pdf" + ) + if problem == "foreign-document": + foreign = FilingDraft.objects.create(jurisdiction="illinois") + document = FilingDocument.objects.create(draft=foreign, role="lead", s3_key="other/private.pdf") + payload["al_court_bundle"][0]["document_id"] = document.pk + with patch("efile.services.efsp_payload.S3UploadHandler") as storage: + with pytest.raises(PayloadValidationError): + prepare_efile_payload( + payload, + "vermont" if problem == "wrong-jurisdiction" else "illinois", + ".." if problem == "invalid-court" else "other" if problem == "wrong-court" else "cook:law1", + draft=None if problem == "no-draft" else draft, + ) + storage.return_value.get_public_url.assert_not_called() + + +@pytest.mark.parametrize("endpoint", ["payment-fees", "submit-final-filing"]) +def test_forwarding_authenticates_session_jurisdiction_not_query(signed_in, endpoint): + client, _ = signed_in + session = client.session + session["jurisdiction"] = "vermont" + session.save() + response = client.post( + f"/api/{endpoint}/?jurisdiction=illinois", + {"efile_data": malicious_payload()}, + content_type="application/json", + HTTP_X_CSRFTOKEN=client.cookies["csrftoken"].value, + ) + assert response.status_code == 401 + + +@pytest.mark.parametrize("endpoint", ["payment-fees", "submit-final-filing"]) +def test_forwarding_requires_court_token(signed_in, endpoint): + client, _ = signed_in + session = client.session + session["auth_tokens"] = {} + session.save() + response = client.post( + f"/api/{endpoint}/", + {"efile_data": malicious_payload()}, + content_type="application/json", + HTTP_X_CSRFTOKEN=client.cookies["csrftoken"].value, + ) + assert response.status_code == 401 + + +def test_empty_draft_cannot_forward_arbitrary_document_url(signed_in): + client, _ = signed_in + with patch("requests.post") as post: + response = client.post( + "/api/payment-fees/", + {"efile_data": malicious_payload()}, + content_type="application/json", + HTTP_X_CSRFTOKEN=client.cookies["csrftoken"].value, + ) + assert response.status_code == 400 + post.assert_not_called() + + +@pytest.mark.parametrize( + "content_type,body,status", + [ + ("text/plain", '{"password":"private-marker"}', 415), + ("application/json", '{"password":"private-marker",', 400), + ("application/json", '["private-marker"]', 400), + ], +) +def test_login_rejects_malformed_inputs_without_echoing_secrets(signed_in, content_type, body, status): + client, _ = signed_in + response = client.post( + "/api/auth/login/", body, content_type=content_type, HTTP_X_CSRFTOKEN=client.cookies["csrftoken"].value + ) + assert response.status_code == status + assert b"private-marker" not in response.content + + +def test_case_lookup_accepts_literal_court_codes_and_uses_query_params(signed_in): + client, _ = signed_in + upstream = Mock(status_code=200) + upstream.json.return_value = [{"caseTrackingID": "dummy-case", "caseTitleText": "A v B"}] + with patch("efile.api.suffolk_api_views.requests.get", return_value=upstream) as get: + response = client.get( + "/api/suffolk/lookup-case/", {"jurisdiction": "illinois", "court": "cook:law1", "caseNumber": "A&B/123"} + ) + assert response.status_code == 200 + assert response.json()["caseInfo"]["caseTrackingID"] == "dummy-case" + assert get.call_args.args[0].endswith("/jurisdictions/illinois/cases/courts/cook:law1/cases") + assert get.call_args.kwargs["params"] == {"docket_number": "A&B/123"} + assert get.call_args.kwargs["allow_redirects"] is False + + +@pytest.mark.parametrize("value", ["..", ".", "%2e%2e", "court/../other", "court?query", "court#fragment"]) +def test_credentialed_service_paths_reject_nonliteral_codes(signed_in, value): + client, draft = signed_in + request = RequestFactory().get("/") + request.user, request.session = draft.user, client.session + with patch("requests.get") as get: + with pytest.raises(ValueError): + fetch_filing_detail(request, "illinois", value, "filing-id") + with pytest.raises(ValueError): + fetch_filing_detail(request, "illinois", "cook:law1", value) + draft.previous_case_id = "case-id" + draft.court_code = value + with pytest.raises(CaseImportError): + fetch_case(draft, "dummy-token") + draft.court_code = "cook:law1" + draft.previous_case_id = value + with pytest.raises(CaseImportError): + fetch_case(draft, "dummy-token") + assert bank_payment_policy("illinois", value) == "unknown" + get.assert_not_called() + + +def test_payment_service_validates_jurisdiction_before_attaching_key(): + with patch("requests.get") as get: + with pytest.raises(ValueError): + payment_accounts("../../api_user_settings/serverid?", "dummy-token") + get.assert_not_called() diff --git a/efile_app/efile/tests/test_durable_drafts.py b/efile_app/efile/tests/test_durable_drafts.py index ba2b3fc3..e64379a7 100644 --- a/efile_app/efile/tests/test_durable_drafts.py +++ b/efile_app/efile/tests/test_durable_drafts.py @@ -37,10 +37,22 @@ def _authorize_jurisdiction_session(client, jurisdiction="illinois"): session.save() -def _prepare_submission(client, draft, jurisdiction="illinois"): +def _prepare_submission(client, draft, monkeypatch, jurisdiction="illinois"): """Populate the draft (the source of truth) and session so submit can run.""" write_case_data(draft, {"court": "cook:cd"}) - write_upload_data(draft, {"files": {"lead": {"url": "https://example.com/petition.pdf"}}}) + write_upload_data( + draft, + { + "files": {"lead": {"url": "https://example.com/petition.pdf", "s3_key": "owned/petition.pdf"}}, + "lead_filing_type": "petition", + "lead_filing_component": "lead", + }, + ) + monkeypatch.setattr( + "efile.services.efsp_payload.S3UploadHandler.get_public_url", + lambda *args, **kwargs: "https://storage.invalid/petition.pdf", + ) + monkeypatch.setattr("efile.services.efsp_payload._EfspLookups.get", lambda *args: []) draft.documents.update(preparation="unchanged", preparation_reviewed_at=timezone.now()) # Submission is only offered against a quote that still prices the filing. record_fee_quote(draft, "0.00", []) @@ -492,11 +504,11 @@ def fake_post(*_args, **_kwargs): user = django_user_model.objects.create_user(username="submit-user", tyler_jurisdiction="illinois") draft = FilingDraft.objects.create(user=user, jurisdiction="illinois", current_step=WorkflowStepKey.REVIEW) client.force_login(user) - _prepare_submission(client, draft) + _prepare_submission(client, draft, monkeypatch) response = client.post( reverse("submit_final_filing"), - data=accepted_submission(draft, efile_data={"al_court_bundle": {}}), + data=accepted_submission(draft, efile_data={"al_court_bundle": [{}]}), content_type="application/json", ) @@ -535,11 +547,11 @@ def fake_post(*_args, **_kwargs): checklist_item_id="petition", ) client.force_login(user) - _prepare_submission(client, draft) + _prepare_submission(client, draft, monkeypatch) response = client.post( reverse("submit_final_filing"), - data=accepted_submission(draft, efile_data={"al_court_bundle": {}}), + data=accepted_submission(draft, efile_data={"al_court_bundle": [{}]}), content_type="application/json", ) @@ -557,11 +569,11 @@ def fake_post(*_args, **_kwargs): user = django_user_model.objects.create_user(username="error-user", tyler_jurisdiction="illinois") draft = FilingDraft.objects.create(user=user, jurisdiction="illinois", current_step=WorkflowStepKey.REVIEW) client.force_login(user) - _prepare_submission(client, draft) + _prepare_submission(client, draft, monkeypatch) response = client.post( reverse("submit_final_filing"), - data=accepted_submission(draft, efile_data={"al_court_bundle": {}}), + data=accepted_submission(draft, efile_data={"al_court_bundle": [{}]}), content_type="application/json", ) @@ -611,12 +623,12 @@ def test_ambiguous_submission_states_are_not_automatically_reclaimed(django_user @pytest.mark.django_db -def test_precondition_failure_releases_claim_to_draft(client, django_user_model): +def test_precondition_failure_releases_claim_to_draft(client, django_user_model, monkeypatch): """A failure before the external call frees the draft for a safe retry.""" user = django_user_model.objects.create_user(username="precondition-user", tyler_jurisdiction="illinois") draft = FilingDraft.objects.create(user=user, jurisdiction="illinois", current_step=WorkflowStepKey.REVIEW) client.force_login(user) - _prepare_submission(client, draft) + _prepare_submission(client, draft, monkeypatch) response = client.post( reverse("submit_final_filing"), @@ -644,11 +656,11 @@ def boom(*_args, **_kwargs): user = django_user_model.objects.create_user(username="ambiguous-user", tyler_jurisdiction="illinois") draft = FilingDraft.objects.create(user=user, jurisdiction="illinois", current_step=WorkflowStepKey.REVIEW) client.force_login(user) - _prepare_submission(client, draft) + _prepare_submission(client, draft, monkeypatch) response = client.post( reverse("submit_final_filing"), - data=accepted_submission(draft, efile_data={"al_court_bundle": {}}), + data=accepted_submission(draft, efile_data={"al_court_bundle": [{}]}), content_type="application/json", ) @@ -658,7 +670,7 @@ def boom(*_args, **_kwargs): retry = client.post( reverse("submit_final_filing"), - data=accepted_submission(draft, efile_data={"al_court_bundle": {}}), + data=accepted_submission(draft, efile_data={"al_court_bundle": [{}]}), content_type="application/json", ) @@ -733,11 +745,11 @@ def unreachable_post(*_args, **_kwargs): user = django_user_model.objects.create_user(username="invalid-payload-user", tyler_jurisdiction="illinois") draft = FilingDraft.objects.create(user=user, jurisdiction="illinois", current_step=WorkflowStepKey.REVIEW) client.force_login(user) - _prepare_submission(client, draft) + _prepare_submission(client, draft, monkeypatch) response = client.post( reverse("submit_final_filing"), - data=accepted_submission(draft, efile_data={"al_court_bundle": {}}), + data=accepted_submission(draft, efile_data={"al_court_bundle": [{}]}), content_type="application/json", ) diff --git a/efile_app/efile/tests/test_efsp_payload.py b/efile_app/efile/tests/test_efsp_payload.py index 954c7b60..dcf6c59b 100644 --- a/efile_app/efile/tests/test_efsp_payload.py +++ b/efile_app/efile/tests/test_efsp_payload.py @@ -14,8 +14,8 @@ from efile.services.efsp_payload import ( PayloadValidationError, _EfspLookups, + normalize_efile_payload, normalize_optional_services, - prepare_efile_payload, resolve_placeholder_filing_components, substitute_test_document_urls, validate_document_selections, @@ -109,7 +109,7 @@ def test_full_payload_preparation_refuses_too(): with override_settings(EFSP_TEST_DOCUMENT_URL=STAND_IN_URL, DEBUG=False): with pytest.raises(ImproperlyConfigured): - prepare_efile_payload(efile_data, "illinois", "adams") + normalize_efile_payload(efile_data, "illinois", "adams") # --- EFSP_TEST_DOCUMENT_URL on in development -------------------------------- @@ -157,7 +157,7 @@ def test_empty_cross_references_is_dropped_rather_than_sent(empty_value): efile_data = {"al_court_bundle": [], "cross_references": empty_value} with override_settings(EFSP_TEST_DOCUMENT_URL=""): - prepare_efile_payload(efile_data, "illinois", "adams") + normalize_efile_payload(efile_data, "illinois", "adams") assert "cross_references" not in efile_data @@ -166,7 +166,7 @@ def test_populated_cross_references_is_preserved(): efile_data = {"al_court_bundle": [], "cross_references": [{"code": "1", "value": "abc"}]} with override_settings(EFSP_TEST_DOCUMENT_URL=""): - prepare_efile_payload(efile_data, "illinois", "adams") + normalize_efile_payload(efile_data, "illinois", "adams") assert efile_data["cross_references"] == [{"code": "1", "value": "abc"}] @@ -219,7 +219,7 @@ def test_preparation_rejects_a_blank_filing_type_end_to_end(): with override_settings(EFSP_TEST_DOCUMENT_URL=""): with pytest.raises(PayloadValidationError, match="filing type"): - prepare_efile_payload(efile_data, "illinois", "edgar") + normalize_efile_payload(efile_data, "illinois", "edgar") # --- placeholder filing components ------------------------------------------- @@ -439,12 +439,12 @@ def fail(*args, **kwargs): def test_preparation_rejects_the_payload_end_to_end(monkeypatch, settings): - """prepare_efile_payload is what both the fee quote and the submission call.""" + """normalize_efile_payload is what both the fee quote and the submission call.""" _party_type_api(monkeypatch, settings, [PLAINTIFF, DEFENDANT]) with override_settings(EFSP_TEST_DOCUMENT_URL=""): with pytest.raises(PayloadValidationError): - prepare_efile_payload(_payload(["173174"], ["173174"]), "illinois", "cook:chd1") + normalize_efile_payload(_payload(["173174"], ["173174"]), "illinois", "cook:chd1") # --- optional services ------------------------------------------------------- @@ -562,7 +562,7 @@ def record(url, **kwargs): } with override_settings(EFSP_TEST_DOCUMENT_URL=""): - prepare_efile_payload(efile_data, "illinois", "adams") + normalize_efile_payload(efile_data, "illinois", "adams") assert len(calls) == len(set(calls)) diff --git a/efile_app/efile/tests/test_efsp_validation.py b/efile_app/efile/tests/test_efsp_validation.py index 93321681..f16b07b6 100644 --- a/efile_app/efile/tests/test_efsp_validation.py +++ b/efile_app/efile/tests/test_efsp_validation.py @@ -11,7 +11,7 @@ from django.forms import ChoiceField from django.urls import reverse -from efile.models import FilingDraft, FilingParty +from efile.models import FilingDocument, FilingDraft, FilingParty from efile.services.current_drafts import CURRENT_DRAFT_SESSION_KEY from efile.services.efsp_errors import describe_efsp_error, efsp_error_problems, error_actions from efile.services.efsp_payload import PayloadValidationError, validate_party_formats @@ -268,7 +268,17 @@ def test_nested_proxy_error_and_name_length_regex_are_parsed(): @pytest.mark.django_db @pytest.mark.parametrize("prevalidate", [True, False]) -def test_fee_errors_return_precise_actions_for_both_local_and_upstream_rejections(client, draft, prevalidate): +def test_fee_errors_return_precise_actions_for_both_local_and_upstream_rejections( + client, draft, prevalidate, monkeypatch +): + FilingDocument.objects.create( + draft=draft, role="lead", s3_key="owned/petition.pdf", filing_type_code="petition", filing_component_code="lead" + ) + monkeypatch.setattr( + "efile.services.efsp_payload.S3UploadHandler.get_public_url", + lambda *args, **kwargs: "https://storage.invalid/petition.pdf", + ) + monkeypatch.setattr("efile.services.efsp_payload._EfspLookups.get", lambda *args: []) party = FilingParty.objects.create(draft=draft, role="other", first_name="Lee", last_name="Smith", party_type="DEF") payload = { "other_parties": [ @@ -278,7 +288,7 @@ def test_fee_errors_return_precise_actions_for_both_local_and_upstream_rejection "address": {"state": "AE" if prevalidate else "MA"}, } ], - "al_court_bundle": [], + "al_court_bundle": [{}], } response = Mock(status_code=400, text="Rejected") response.json.return_value = {"wrong_vars": [{"name": "other_parties[0].name.last", "currentVal": "Smith"}]} diff --git a/efile_app/efile/tests/test_filing_availability.py b/efile_app/efile/tests/test_filing_availability.py index 80a4545d..5744445a 100644 --- a/efile_app/efile/tests/test_filing_availability.py +++ b/efile_app/efile/tests/test_filing_availability.py @@ -165,6 +165,18 @@ def test_direct_submit_cannot_bypass_new_restriction(configure, client, submissi @pytest.mark.django_db def test_outgoing_ids_are_resolved_to_names_before_checking(configure, client, submission_draft, monkeypatch): configure({"rules": [{"case_types": ["Contract"], "filing_types": ["Blocked filing"]}]}) + submission_draft.documents.update(s3_key="owned/lead.pdf", filing_type_code="allowed", filing_component_code="lead") + FilingDocument.objects.create( + draft=submission_draft, + role="supporting", + s3_key="owned/supporting.pdf", + filing_type_code="blocked", + filing_component_code="lead", + ) + monkeypatch.setattr( + "efile.services.efsp_payload.S3UploadHandler.get_public_url", + lambda *args, **kwargs: "https://storage.invalid/document.pdf", + ) request = RequestFactory().post("/") request.user = submission_draft.user request.session = client.session diff --git a/efile_app/efile/tests/test_filing_integrity.py b/efile_app/efile/tests/test_filing_integrity.py index eb247b19..d13d02cd 100644 --- a/efile_app/efile/tests/test_filing_integrity.py +++ b/efile_app/efile/tests/test_filing_integrity.py @@ -216,11 +216,11 @@ def test_primary_type_tracks_edits_clearing_and_lead_deletion(draft): assert (draft.filing_type_code, draft.filing_type_name) == ("", "") -def test_organization_and_submission_preserve_primary_type_and_confirmation_identity(draft): +def test_organization_and_submission_preserve_primary_type_and_confirmation_identity(draft, monkeypatch): from efile.tests.test_durable_drafts import FakeApiResponse, _prepare_submission client = signed_in(draft.user) - _prepare_submission(client, draft) + _prepare_submission(client, draft, monkeypatch) draft.refresh_from_db() draft.document_checklist_acknowledged = True draft.save() @@ -245,7 +245,7 @@ def test_organization_and_submission_preserve_primary_type_and_confirmation_iden with patch("requests.post", return_value=FakeApiResponse(201, {"filing_id": "adoption-123"})): response = client.post( reverse("submit_final_filing") + f"?draft={draft.pk}", - accepted_submission(draft, efile_data={"al_court_bundle": {}}), + accepted_submission(draft, efile_data={"al_court_bundle": [{}]}), content_type="application/json", ) assert response.status_code == 200 diff --git a/efile_app/efile/tests/tests.py b/efile_app/efile/tests/tests.py index 3bf64042..a516d5af 100644 --- a/efile_app/efile/tests/tests.py +++ b/efile_app/efile/tests/tests.py @@ -250,12 +250,12 @@ def test_profile_api_includes_location_data(self, client, user): assert "preferred_county" not in data["data"] def test_profile_api_unauthenticated_user(self, client): - """Test profile API handles unauthenticated users with demo data.""" + """Private account data requires an authenticated court identity.""" response = client.get( "/api/auth/profile/", {"jurisdiction": "illinois"}, HTTP_X_REQUESTED_WITH="XMLHttpRequest" ) - assert response.status_code == 500 + assert response.status_code == 401 # ============================================================================ diff --git a/efile_app/efile/utils/efsp_security.py b/efile_app/efile/utils/efsp_security.py new file mode 100644 index 00000000..480877b3 --- /dev/null +++ b/efile_app/efile/utils/efsp_security.py @@ -0,0 +1,50 @@ +"""Validate input before constructing credentialed EFSP requests.""" + +import re +from functools import partial, wraps + +from django.http import JsonResponse +from django.views.decorators.cache import never_cache + +from efile.utils.config_loader import config_loader +from efile.utils.jurisdiction_stuff import get_jurisdiction_from_request, has_jurisdiction_login + + +def validated_jurisdiction(value): + if not isinstance(value, str) or value.lower() not in config_loader.get_available_jurisdictions(): + raise ValueError("Choose a supported jurisdiction.") + return value.lower() + + +def validated_code(value): + """Accept one literal code, including Tyler's colon-separated court codes. + + Reject percent escapes as well as URL delimiters so neither Requests nor + a downstream router can reinterpret a code as a different path. + """ + value = str(value or "") + if value in {".", ".."} or not re.fullmatch(r"[A-Za-z0-9_:.-]+", value): + raise ValueError("Invalid court or filing code.") + return value + + +def require_efsp_login(view=None, *, session_jurisdiction=False): + """Private proxy APIs require the active account's jurisdiction and token.""" + + if view is None: + return partial(require_efsp_login, session_jurisdiction=session_jurisdiction) + + @wraps(view) + @never_cache + def wrapped(request, *args, **kwargs): + try: + jurisdiction = validated_jurisdiction( + request.session.get("jurisdiction") if session_jurisdiction else get_jurisdiction_from_request(request) + ) + except ValueError as error: + return JsonResponse({"success": False, "error": str(error)}, status=400) + if not has_jurisdiction_login(request, jurisdiction): + return JsonResponse({"success": False, "error": "Sign in to this jurisdiction first."}, status=401) + return view(request, *args, **kwargs) + + return wrapped diff --git a/efile_app/efile/views/session_api.py b/efile_app/efile/views/session_api.py index 093961e6..a76e9102 100644 --- a/efile_app/efile/views/session_api.py +++ b/efile_app/efile/views/session_api.py @@ -3,7 +3,6 @@ from django.conf import settings from django.http import JsonResponse -from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_http_methods from ..services.current_drafts import get_current_draft @@ -245,6 +244,7 @@ def forward_final_filing(request, data): api_url, json=efile_data, headers=headers, + allow_redirects=False, timeout=( settings.EFILE_SUBMISSION_CONNECT_TIMEOUT_SECONDS, settings.EFILE_SUBMISSION_READ_TIMEOUT_SECONDS, @@ -294,7 +294,6 @@ def forward_final_filing(request, data): return JsonResponse({"success": False, "error": f"An error occurred during submission: {str(e)}"}, status=500) -@csrf_exempt @require_http_methods(["POST"]) def clear_session_data(request): """Clear all session data for testing purposes.""" @@ -302,7 +301,6 @@ def clear_session_data(request): return JsonResponse({"success": True, "message": "Session data cleared"}) -@csrf_exempt @require_http_methods(["POST"]) def api_save_case_data(request): """ @@ -359,7 +357,6 @@ def api_save_case_data(request): return JsonResponse({"success": False, "error": str(e)}, status=500) -@csrf_exempt @require_http_methods(["POST"]) def fetch_and_save_party_type(request): """ @@ -452,7 +449,6 @@ def fetch_and_save_party_type(request): return JsonResponse({"success": False, "error": str(e)}, status=500) -@csrf_exempt @require_http_methods(["POST"]) def save_party_type_to_session(request): """ diff --git a/efile_app/efile/views/submission.py b/efile_app/efile/views/submission.py index 1833249f..15dd0517 100644 --- a/efile_app/efile/views/submission.py +++ b/efile_app/efile/views/submission.py @@ -5,7 +5,6 @@ from django.db import transaction from django.http import JsonResponse from django.utils import timezone -from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_http_methods from efile.api.suffolk_api_views import get_tyler_token @@ -18,6 +17,7 @@ from efile.services.filing_availability import draft_unavailable_message from efile.services.filing_plans import mark_attached_items_filed from efile.services.submission_errors import PRE_SUBMIT_ERROR_CODES, SubmissionErrorCode +from efile.utils.efsp_security import require_efsp_login from .confirmation import LAST_SUBMITTED_DRAFT_SESSION_KEY from .session_api import forward_final_filing @@ -100,8 +100,8 @@ def _confirmed_api_rejection(payload: dict) -> bool: return isinstance(code, int) and 400 <= code < 500 and code not in (408, 409) -@csrf_exempt @require_http_methods(["POST"]) +@require_efsp_login(session_jurisdiction=True) def submit_final_filing(request): """Submit through the session path, guarding against duplicate external filings."""