From b6733703abf7604872ce19fd7a4d8845b864ae16 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 5 Jun 2026 18:41:55 -0400 Subject: [PATCH 1/2] Add admin summary-PDF download endpoint and button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The weekly summary PDF generated at Thursday close was stored in the reports bucket with no way to reach it from the UI — admins had to pull it from S3 manually. Surface it in the admin panel: - submit_order: GET /api/admin/summary-pdf?week= (admin-gated) returns a 5-minute presigned URL from the SUMMARY item's stamped PDF key; 404 for weeks that haven't closed. - template.yaml: REPORTS_BUCKET env var + read-only s3:GetObject on reports/* for SubmitOrderFunction (needed so the presigned URL is signed with sufficient permissions). - generate_form.py: "Download summary PDF" button in the admin header; explains Thursday-close timing on 404. - Tests: presign happy path, 404 open week, 400 missing week, 401 unauthenticated. - README updated. --- README.md | 4 +- functions/submit_order/handler.py | 29 ++++++++++ src/server/generate_form.py | 27 ++++++++++ template.yaml | 12 +++++ tests/test_submit_order.py | 89 +++++++++++++++++++++++++++++++ 5 files changed, 160 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 5bc887d..679b5c3 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ who haven't ordered |-----|----------| | `reports/{week}/order-summary.csv` | Meal-level aggregate (meal, qty, unit price, line total) for the Redefine order | | `reports/{week}/payroll-deductions.csv` | Per-employee payroll deduction totals | -| `reports/{week}/weekly-summary-{week}.pdf` | Per-person summary (employee → item → quantity, **no pricing**); stored only, not emailed | +| `reports/{week}/weekly-summary-{week}.pdf` | Per-person summary (employee → item → quantity, **no pricing**); downloadable from the admin panel via presigned URL | ## AWS Resources @@ -69,6 +69,7 @@ Admins (configured in DynamoDB `CONFIG/SETTINGS` → `admin_emails` list) get an - Edit order quantities, add new menu items, remove items - Delete orders entirely - **Download order list** — a CSV rollup of item → total quantity across all employees (no per-employee breakdown, no prices) to drive the bulk Redefine order. Generated client-side from the loaded week, so it works for open weeks too. +- **Download summary PDF** — fetches a short-lived presigned URL for the week's per-person summary PDF (generated at Thursday close) and opens it. Returns 404 for weeks that haven't closed yet. All admin operations enforce server-side price recalculation from the menu. @@ -80,6 +81,7 @@ All admin operations enforce server-side price recalculation from the menu. | GET | `/api/admin/orders?week=YYYY-WNN` | Get all orders for a week | | PUT | `/api/admin/orders` | Update an order (recalculates prices) | | DELETE | `/api/admin/orders?week=...&email=...` | Delete an order | +| GET | `/api/admin/summary-pdf?week=YYYY-WNN` | Presigned URL for the week's summary PDF (404 if week not closed) | ## Setup diff --git a/functions/submit_order/handler.py b/functions/submit_order/handler.py index c90fcae..c2c3515 100644 --- a/functions/submit_order/handler.py +++ b/functions/submit_order/handler.py @@ -21,6 +21,7 @@ from shared.db import ( get_orders, get_roster, get_settings, + get_summary, list_weeks, put_order, ) @@ -191,6 +192,9 @@ def lambda_handler(event, context): return handle_admin_update(event) return handle_admin_orders(event) + if "/admin/summary-pdf" in path: + return handle_admin_summary_pdf(event) + if "/form-status/" in path: return handle_form_status(event) @@ -250,6 +254,31 @@ def handle_admin_orders(event): ) +def handle_admin_summary_pdf(event): + """Return a short-lived presigned URL for the week's summary PDF.""" + user, err = _verify_admin(event) + if err: + return err + + qs = event.get("queryStringParameters") or {} + week = qs.get("week", "") + if not week: + return response(400, {"error": "week query param is required"}) + + summary = get_summary(week) + pdf_key = (summary or {}).get("weekly_summary_pdf_s3_key") + if not pdf_key: + return response(404, {"error": "No summary PDF available for this week"}) + + url = boto3.client("s3").generate_presigned_url( + "get_object", + Params={"Bucket": os.environ["REPORTS_BUCKET"], "Key": pdf_key}, + ExpiresIn=300, + ) + logger.info("Admin %s requested summary PDF for %s", user["email"], week) + return response(200, {"week": week, "url": url}) + + def handle_admin_delete(event): user, err = _verify_admin(event) if err: diff --git a/src/server/generate_form.py b/src/server/generate_form.py index 6d5e853..567ea57 100644 --- a/src/server/generate_form.py +++ b/src/server/generate_form.py @@ -60,6 +60,9 @@ def generate_form( admin_url = ( f"{api_url}/api/admin/orders" if api_url else "/api/admin/orders" ).replace(" 0 or company_subsidy > 0 use_google_auth = bool(google_client_id) @@ -416,6 +419,7 @@ body {{ padding-bottom: 80px; }}
+
@@ -452,6 +456,7 @@ const SUBMIT_URL = '{submit_url}'; const STATUS_URL = '{status_url}'; const ROSTER_URL = '{roster_url}'; const ADMIN_URL = '{admin_url}'; +const ADMIN_PDF_URL = '{admin_pdf_url}'; const API_KEY = {api_key_json}; const WEEK = '{week}'; const BULK_DISCOUNT = {bulk_discount}; @@ -838,6 +843,28 @@ function downloadOrderList() {{ URL.revokeObjectURL(url); }} +// Fetch a short-lived presigned URL for the loaded week's summary PDF +// (generated at Thursday close) and open it. 404 means the week has not +// closed yet, so no PDF exists. +function downloadSummaryPdf() {{ + const week = currentAdminWeek || WEEK; + const btn = document.getElementById('admin-pdf-btn'); + if (btn) btn.disabled = true; + fetch(ADMIN_PDF_URL + '?week=' + encodeURIComponent(week), {{ headers: adminHeaders() }}) + .then(r => r.json().then(data => ({{ ok: r.ok, status: r.status, data }}))) + .then(({{ ok, status, data }}) => {{ + if (ok && data.url) {{ + window.location.href = data.url; + }} else if (status === 404) {{ + alert('No summary PDF for ' + week + ' yet — it is generated when the week closes on Thursday.'); + }} else {{ + alert('Could not fetch the summary PDF: ' + (data.error || 'unknown error')); + }} + }}) + .catch(() => alert('Could not fetch the summary PDF.')) + .finally(() => {{ if (btn) btn.disabled = false; }}); +}} + let lastAdminData = null; const origLoadWeekOrders = loadWeekOrders; diff --git a/template.yaml b/template.yaml index 3f1b36f..a66df50 100644 --- a/template.yaml +++ b/template.yaml @@ -257,6 +257,7 @@ Resources: FORM_APIKEY_SM_NAME: meal-order-manager/form-api-key SLACK_NOTIFIER_ARN: !GetAtt SlackNotifierFunction.Arn GOOGLE_CLIENT_ID_PARAM: /meal-order-manager/google-client-id + REPORTS_BUCKET: !Ref ReportsBucket Policies: - DynamoDBCrudPolicy: TableName: !Ref OrdersTable @@ -270,6 +271,11 @@ Resources: - Effect: Allow Action: ssm:GetParameter Resource: !Sub 'arn:aws:ssm:${AWS::Region}:${AWS::AccountId}:parameter/meal-order-manager/*' + # Read-only access to weekly report PDFs for the admin + # summary-pdf presigned-URL endpoint. + - Effect: Allow + Action: s3:GetObject + Resource: !Sub '${ReportsBucket.Arn}/reports/*' Events: SubmitOrder: Type: HttpApi @@ -307,6 +313,12 @@ Resources: ApiId: !Ref OrderApi Path: /api/admin/orders Method: DELETE + AdminSummaryPdf: + Type: HttpApi + Properties: + ApiId: !Ref OrderApi + Path: /api/admin/summary-pdf + Method: GET CloseFormFunction: Type: AWS::Serverless::Function diff --git a/tests/test_submit_order.py b/tests/test_submit_order.py index 9caeaf1..de1e976 100644 --- a/tests/test_submit_order.py +++ b/tests/test_submit_order.py @@ -1584,3 +1584,92 @@ def test_slack_failure_does_not_fail_order( assert status == 200, f"Expected 200 despite Slack failure, got {status}: {body}" assert body["status"] == "ok", f"Expected status='ok', got '{body['status']}'" mock_put.assert_called_once() + + +# --------------------------------------------------------------------------- +# Admin summary-PDF endpoint +# --------------------------------------------------------------------------- +def _pdf_event(week=None): + qs = {"week": week} if week else {} + return { + "rawPath": "/api/admin/summary-pdf", + "requestContext": {"http": {"method": "GET"}}, + "queryStringParameters": qs, + "headers": {"authorization": "Bearer admin-token"}, + } + + +@patch.dict(os.environ, {"REPORTS_BUCKET": "test-reports-bucket"}) +@patch("submit_order_handler.boto3.client") +@patch("submit_order_handler.get_summary") +@patch( + "submit_order_handler._verify_admin", + return_value=({"email": "adam@seahavenind.com"}, None), +) +def test_admin_summary_pdf_returns_presigned_url( + mock_verify, mock_get_summary, mock_boto_client +): + """Closed week with a stamped PDF key returns a presigned URL.""" + from submit_order_handler import lambda_handler + + mock_get_summary.return_value = { + "weekly_summary_pdf_s3_key": "reports/2026-W22/weekly-summary-2026-W22.pdf" + } + mock_s3 = MagicMock() + mock_s3.generate_presigned_url.return_value = "https://signed.example/pdf" + mock_boto_client.return_value = mock_s3 + + status, body = _parse_response(lambda_handler(_pdf_event("2026-W22"), None)) + + assert status == 200, f"Expected 200, got {status}: {body}" + assert body["url"] == "https://signed.example/pdf" + assert body["week"] == "2026-W22" + mock_s3.generate_presigned_url.assert_called_once_with( + "get_object", + Params={ + "Bucket": "test-reports-bucket", + "Key": "reports/2026-W22/weekly-summary-2026-W22.pdf", + }, + ExpiresIn=300, + ) + + +@patch("submit_order_handler.get_summary", return_value=None) +@patch( + "submit_order_handler._verify_admin", + return_value=({"email": "adam@seahavenind.com"}, None), +) +def test_admin_summary_pdf_404_when_week_not_closed(mock_verify, mock_get_summary): + """No SUMMARY item (week still open) returns 404.""" + from submit_order_handler import lambda_handler + + status, body = _parse_response(lambda_handler(_pdf_event("2026-W23"), None)) + + assert status == 404, f"Expected 404, got {status}: {body}" + assert "no summary pdf" in body["error"].lower() + + +@patch( + "submit_order_handler._verify_admin", + return_value=({"email": "adam@seahavenind.com"}, None), +) +def test_admin_summary_pdf_400_without_week(mock_verify): + """Missing week query param returns 400.""" + from submit_order_handler import lambda_handler + + status, body = _parse_response(lambda_handler(_pdf_event(), None)) + + assert status == 400, f"Expected 400, got {status}: {body}" + + +@patch( + "submit_order_handler._verify_admin", + return_value=(None, submit_order_handler.response(401, {"error": "Missing token"})), +) +def test_admin_summary_pdf_requires_admin(mock_verify): + """Auth failure from _verify_admin is returned as-is.""" + from submit_order_handler import lambda_handler + + status, body = _parse_response(lambda_handler(_pdf_event("2026-W22"), None)) + + assert status == 401, f"Expected 401, got {status}: {body}" From f2a0692577b526801ccfb167a0dc127306e9785d Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 5 Jun 2026 18:44:25 -0400 Subject: [PATCH 2/2] Harden summary-PDF endpoint per cross-review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address GPT-4.1 cross-review of the IAM change: - Validate the week param (YYYY-WNN) and the DynamoDB-sourced PDF key shape before presigning, so a tampered SUMMARY record can't mint URLs for other report files (payroll CSVs). - Narrow the IAM resource from reports/* to reports/*/weekly-summary-*.pdf — least privilege over the bucket. - Reuse a module-level S3 client; name the URL TTL constant. - Distinguish "week not found" from "PDF key missing" 404s. - Tests: malformed-week 400 and tampered-key 500 (asserts no presign). --- functions/submit_order/handler.py | 25 +++++++++++++++----- template.yaml | 7 +++--- tests/test_submit_order.py | 39 +++++++++++++++++++++++++++---- 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/functions/submit_order/handler.py b/functions/submit_order/handler.py index c2c3515..e48d871 100644 --- a/functions/submit_order/handler.py +++ b/functions/submit_order/handler.py @@ -1,6 +1,7 @@ import json import logging import os +import re import sys import time import urllib.error @@ -34,6 +35,7 @@ if not logger.handlers: EASTERN = ZoneInfo("America/New_York") CACHE_TTL_SECONDS = 300 # 5-minute TTL for cached config values +PRESIGNED_URL_TTL_SECONDS = 300 # summary-PDF presigned URL lifetime ALLOWED_DOMAINS = {"seahavenind.com", "seahaven.com"} @@ -48,6 +50,7 @@ _settings_ts = 0.0 _google_client_id = None _google_client_id_ts = 0.0 _lambda = boto3.client("lambda") +_s3 = boto3.client("s3") def _get_api_key() -> str: @@ -262,18 +265,28 @@ def handle_admin_summary_pdf(event): qs = event.get("queryStringParameters") or {} week = qs.get("week", "") - if not week: - return response(400, {"error": "week query param is required"}) + if not re.match(r"^\d{4}-W\d{2}$", week): + return response(400, {"error": "week query param must be YYYY-WNN"}) summary = get_summary(week) - pdf_key = (summary or {}).get("weekly_summary_pdf_s3_key") - if not pdf_key: + if summary is None: + return response(404, {"error": "No summary PDF: week not found or not closed"}) + pdf_key = summary.get("weekly_summary_pdf_s3_key", "") + # The key comes from a DynamoDB record; only presign keys matching the + # shape aggregate_orders writes, so a tampered record can't expose other + # report files (e.g. payroll CSVs). + if not re.match( + r"^reports/\d{4}-W\d{2}/weekly-summary-\d{4}-W\d{2}\.pdf$", pdf_key + ): + if pdf_key: + logger.error("Unexpected summary PDF key shape for %s: %s", week, pdf_key) + return response(500, {"error": "Internal error"}) return response(404, {"error": "No summary PDF available for this week"}) - url = boto3.client("s3").generate_presigned_url( + url = _s3.generate_presigned_url( "get_object", Params={"Bucket": os.environ["REPORTS_BUCKET"], "Key": pdf_key}, - ExpiresIn=300, + ExpiresIn=PRESIGNED_URL_TTL_SECONDS, ) logger.info("Admin %s requested summary PDF for %s", user["email"], week) return response(200, {"week": week, "url": url}) diff --git a/template.yaml b/template.yaml index a66df50..aa07aec 100644 --- a/template.yaml +++ b/template.yaml @@ -271,11 +271,12 @@ Resources: - Effect: Allow Action: ssm:GetParameter Resource: !Sub 'arn:aws:ssm:${AWS::Region}:${AWS::AccountId}:parameter/meal-order-manager/*' - # Read-only access to weekly report PDFs for the admin - # summary-pdf presigned-URL endpoint. + # Read-only access to weekly summary PDFs (only — not the + # payroll/order CSVs) for the admin summary-pdf presigned-URL + # endpoint. - Effect: Allow Action: s3:GetObject - Resource: !Sub '${ReportsBucket.Arn}/reports/*' + Resource: !Sub '${ReportsBucket.Arn}/reports/*/weekly-summary-*.pdf' Events: SubmitOrder: Type: HttpApi diff --git a/tests/test_submit_order.py b/tests/test_submit_order.py index de1e976..47a9037 100644 --- a/tests/test_submit_order.py +++ b/tests/test_submit_order.py @@ -1600,14 +1600,14 @@ def _pdf_event(week=None): @patch.dict(os.environ, {"REPORTS_BUCKET": "test-reports-bucket"}) -@patch("submit_order_handler.boto3.client") +@patch("submit_order_handler._s3") @patch("submit_order_handler.get_summary") @patch( "submit_order_handler._verify_admin", return_value=({"email": "adam@seahavenind.com"}, None), ) def test_admin_summary_pdf_returns_presigned_url( - mock_verify, mock_get_summary, mock_boto_client + mock_verify, mock_get_summary, mock_s3 ): """Closed week with a stamped PDF key returns a presigned URL.""" from submit_order_handler import lambda_handler @@ -1615,9 +1615,7 @@ def test_admin_summary_pdf_returns_presigned_url( mock_get_summary.return_value = { "weekly_summary_pdf_s3_key": "reports/2026-W22/weekly-summary-2026-W22.pdf" } - mock_s3 = MagicMock() mock_s3.generate_presigned_url.return_value = "https://signed.example/pdf" - mock_boto_client.return_value = mock_s3 status, body = _parse_response(lambda_handler(_pdf_event("2026-W22"), None)) @@ -1662,6 +1660,39 @@ def test_admin_summary_pdf_400_without_week(mock_verify): assert status == 400, f"Expected 400, got {status}: {body}" +@patch( + "submit_order_handler._verify_admin", + return_value=({"email": "adam@seahavenind.com"}, None), +) +def test_admin_summary_pdf_400_malformed_week(mock_verify): + """Week not matching YYYY-WNN returns 400 before any lookup.""" + from submit_order_handler import lambda_handler + + status, body = _parse_response(lambda_handler(_pdf_event("../../etc"), None)) + + assert status == 400, f"Expected 400, got {status}: {body}" + + +@patch("submit_order_handler._s3") +@patch("submit_order_handler.get_summary") +@patch( + "submit_order_handler._verify_admin", + return_value=({"email": "adam@seahavenind.com"}, None), +) +def test_admin_summary_pdf_rejects_tampered_key(mock_verify, mock_get_summary, mock_s3): + """A PDF key outside the expected shape is never presigned (500, no URL).""" + from submit_order_handler import lambda_handler + + mock_get_summary.return_value = { + "weekly_summary_pdf_s3_key": "reports/2026-W22/payroll-deductions.csv" + } + + status, body = _parse_response(lambda_handler(_pdf_event("2026-W22"), None)) + + assert status == 500, f"Expected 500 for tampered key, got {status}: {body}" + mock_s3.generate_presigned_url.assert_not_called() + + @patch( "submit_order_handler._verify_admin", return_value=(None, submit_order_handler.response(401, {"error": "Missing token"})),