diff --git a/.github/workflows/weekly-menu.yml b/.github/workflows/weekly-menu.yml index a65d28f..d465940 100644 --- a/.github/workflows/weekly-menu.yml +++ b/.github/workflows/weekly-menu.yml @@ -88,16 +88,6 @@ jobs: echo "dist_id=$DIST_ID" >> $GITHUB_OUTPUT echo "form_url=$FORM_URL" >> $GITHUB_OUTPUT - - name: Get API key - if: env.SKIP_RUN != 'true' - id: apikey - run: | - API_KEY=$(aws secretsmanager get-secret-value \ - --secret-id meal-order-manager/form-api-key \ - --query 'SecretString' --output text) - echo "::add-mask::$API_KEY" - echo "api_key=$API_KEY" >> $GITHUB_OUTPUT - - name: Get discount settings if: env.SKIP_RUN != 'true' id: discount @@ -126,9 +116,10 @@ jobs: GOOGLE_CLIENT_ID=$(aws ssm get-parameter \ --name /meal-order-manager/google-client-id \ --query 'Parameter.Value' \ - --output text 2>/dev/null || echo "") + --output text) if [ "$GOOGLE_CLIENT_ID" = "None" ] || [ -z "$GOOGLE_CLIENT_ID" ]; then - GOOGLE_CLIENT_ID="" + echo "Google client ID is required for cloud form generation" >&2 + exit 1 fi echo "client_id=$GOOGLE_CLIENT_ID" >> $GITHUB_OUTPUT @@ -137,10 +128,9 @@ jobs: run: | python3 src/server/generate_form.py \ --api-url "${{ steps.stack.outputs.api_url }}" \ - --api-key "${{ steps.apikey.outputs.api_key }}" \ --bulk-discount "${{ steps.discount.outputs.bulk_discount }}" \ --company-subsidy "${{ steps.discount.outputs.company_subsidy }}" \ - ${{ steps.google.outputs.client_id && format('--google-client-id "{0}"', steps.google.outputs.client_id) || '' }} + --google-client-id "${{ steps.google.outputs.client_id }}" - name: Upload menu to DynamoDB if: env.SKIP_RUN != 'true' diff --git a/.security-review/suppressions.json b/.security-review/suppressions.json index 83ea9f9..bd04a35 100644 --- a/.security-review/suppressions.json +++ b/.security-review/suppressions.json @@ -2,7 +2,7 @@ "suppressions": [ { "id": "gitleaks-generic-api-key-45", - "justification": "False positive. tests/test_submit_order.py:45 defines a low-entropy dummy test constant used as the mocked get_secret return value and injected as the x-api-key header in the test harness (the module makes no real AWS calls). Not a live credential; the real key lives in Secrets Manager (FORM_APIKEY_SM_NAME). Verified proof-or-kill 2026-07-13. Moved from machine-level to repo-local so the Open SWE daily-report automation resolves it." + "justification": "False positive. tests/test_submit_order.py defines a synthetic Google OAuth audience used only for mocked token verification. OAuth client IDs are public identifiers, this fixture is not a credential, and the tests make no real Google or AWS calls." } ] } diff --git a/README.md b/README.md index 922791b..46d001d 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,8 @@ Stack name: `meal-order-manager` (us-east-1) - **API Gateway** — HttpApi for order submission and admin operations - **Lambda** — 7 functions: submit-order, admin-authorizer, close-form, aggregate-orders, slack-notifier, sync-roster, email-report - **EventBridge** — scheduled rules (dual EST/EDT) for close, reminders, payroll email -- **Secrets Manager** — Slack bot token, form API key +- **Secrets Manager** — Slack bot token + - Migration note: the existing `meal-order-manager/form-api-key` secret remains until this change is deployed and verified, then must be deleted during post-deploy cleanup. - **SES** — payroll deduction emails - **CloudWatch Alarms** — coverage across the stack, all notifying the shared `site-alerts` SNS topic (see Monitoring) @@ -80,11 +81,11 @@ sit in ALARM between runs). Alarm names follow `meal-order-manager-- at the table-only dimension, so those are intentionally not alarmed. - **API Gateway (OrderApi, HTTP API v2)** — 5xx (threshold 0), 4xx (threshold 20, 3/2 datapoints to absorb routine 401s from the token authorizer), and p99 Latency - (~3000ms). + (~3000ms). The submit route is limited to 5 requests/second with a burst of 10. ## Authentication -Google Identity Services (OAuth) with tokeninfo endpoint verification. Accepts both `seahavenind.com` and `seahaven.com` Google Workspace domains. +Google Identity Services (OAuth) with tokeninfo endpoint verification. Accepts both `seahavenind.com` and `seahaven.com` Google Workspace domains. Cloud form generation and Lambda order submission fail closed unless `/meal-order-manager/google-client-id` is configured. The local Flask workflow can still use manual name and email entry when Google auth is not configured. ## Admin Panel @@ -137,7 +138,7 @@ sam deploy ### Post-deploy 1. Create the Slack bot token secret: `aws secretsmanager create-secret --name meal-order-manager/slack-bot-token --secret-string "xoxb-..."` -2. Create the form API key secret: `aws secretsmanager create-secret --name meal-order-manager/form-api-key --secret-string "$(openssl rand -hex 32)"` +2. Set the Google OAuth client ID: `aws ssm put-parameter --name /meal-order-manager/google-client-id --type String --value "" --overwrite` 3. Update the Slack channel SSM parameter: `aws ssm put-parameter --name /meal-order-manager/slack-channel-id --value "C0XXXXXXX" --overwrite` 4. Verify SES sender identity for `adam@seahavenind.com` 5. Set up DNS: CNAME `orders.seahaven.com` → CloudFront distribution domain @@ -160,6 +161,7 @@ python3 src/aggregator/aggregate.py # generate CSV reports - `menu_url` — Redefine Meals menu URL - `order_deadline` — displayed on the form - `roster` — employee list (name, email, slack_user_id) +- `google_client_id` — optional locally; required for cloud generation - `output_dir` / `orders_dir` — local output paths ## Project Structure diff --git a/functions/submit_order/handler.py b/functions/submit_order/handler.py index e48d871..d6c6276 100644 --- a/functions/submit_order/handler.py +++ b/functions/submit_order/handler.py @@ -26,7 +26,7 @@ from shared.db import ( list_weeks, put_order, ) -from shared.secrets import get_parameter, get_secret +from shared.secrets import get_parameter logger = logging.getLogger(__name__) logger.setLevel(logging.INFO) @@ -44,7 +44,6 @@ def _eastern_now() -> _dt.datetime: return _dt.datetime.now(EASTERN) -_api_key = None _settings = None _settings_ts = 0.0 _google_client_id = None @@ -53,13 +52,6 @@ _lambda = boto3.client("lambda") _s3 = boto3.client("s3") -def _get_api_key() -> str: - global _api_key - if _api_key is None: - _api_key = get_secret(os.environ["FORM_APIKEY_SM_NAME"]) - return _api_key - - def _get_discount_settings() -> tuple[Decimal, Decimal]: global _settings, _settings_ts now = time.monotonic() @@ -73,6 +65,10 @@ def _get_discount_settings() -> tuple[Decimal, Decimal]: return _settings +def _get_admin_emails() -> set[str]: + return {email.lower() for email in get_settings().get("admin_emails", [])} + + def _google_auth_configured() -> bool: return bool(os.environ.get("GOOGLE_CLIENT_ID_PARAM", "")) @@ -155,6 +151,10 @@ def _verify_google_token(token: str) -> tuple[dict | None, str]: return None, "invalid" +def _authentication_service_unavailable() -> dict: + return response(503, {"error": "Authentication service temporarily unavailable"}) + + def _verify_admin(event) -> tuple[dict | None, dict | None]: """Verify Google auth and admin access. Returns (user_info, error_response).""" if not _google_auth_configured(): @@ -171,14 +171,11 @@ def _verify_admin(event) -> tuple[dict | None, dict | None]: user_info, status = _verify_google_token(token) if status == "unavailable": - return None, response( - 503, {"error": "Authentication service temporarily unavailable"} - ) + return None, _authentication_service_unavailable() if user_info is None: return None, response(403, {"error": "Invalid or unauthorized Google account"}) - admin_emails = {e.lower() for e in get_settings().get("admin_emails", [])} - if user_info["email"].lower() not in admin_emails: + if user_info["email"].lower() not in _get_admin_emails(): return None, response(403, {"error": "Admin access required"}) return user_info, None @@ -417,48 +414,32 @@ def handle_roster(): def handle_submit(event): - api_key = event.get("headers", {}).get("x-api-key", "") - if api_key != _get_api_key(): - return response(403, {"error": "Invalid API key"}) - try: body = json.loads(event.get("body", "{}")) except json.JSONDecodeError: return response(400, {"error": "Invalid JSON"}) - # --- Authentication --- - # If Google auth is configured (SSM param contains a client ID), require a valid - # google_id_token. Manual fallback is only allowed when auth is NOT configured. - # If SSM fetch fails for any other reason, fail closed (503). + # The Lambda is the cloud submission path, so Google authentication is always + # required. Local/manual submissions are handled only by src/server/app.py. google_token = body.get("google_id_token") - if _google_auth_configured(): - try: - client_id = _get_google_client_id() - except Exception as exc: - logger.error("SSM fetch failed for Google client ID: %s", exc) - return response( - 503, {"error": "Authentication service temporarily unavailable"} - ) - if not client_id: - logger.error("Google auth configured but client ID is empty") - return response( - 503, {"error": "Authentication service temporarily unavailable"} - ) - if not google_token: - return response(403, {"error": "Google authentication is required"}) - user_info, verify_status = _verify_google_token(google_token) - if verify_status == "unavailable": - return response( - 503, {"error": "Authentication service temporarily unavailable"} - ) - if user_info is None: - return response(403, {"error": "Invalid or unauthorized Google account"}) - name = user_info["name"] - email = user_info["email"] - else: - name = body.get("employee_name", "").strip() - email = body.get("employee_email", "").strip() + try: + client_id = _get_google_client_id() + except Exception as exc: + logger.error("SSM fetch failed for Google client ID: %s", exc) + return _authentication_service_unavailable() + if not client_id: + logger.error("Google client ID is unavailable") + return _authentication_service_unavailable() + if not google_token: + return response(403, {"error": "Google authentication is required"}) + user_info, verify_status = _verify_google_token(google_token) + if verify_status == "unavailable": + return _authentication_service_unavailable() + if user_info is None: + return response(403, {"error": "Invalid or unauthorized Google account"}) + name = user_info["name"] + email = user_info["email"] items = body.get("items", []) @@ -471,10 +452,7 @@ def handle_submit(event): week = current_week() status = get_form_status(week) - is_admin_user = False - if _google_auth_configured(): - admin_emails = {e.lower() for e in get_settings().get("admin_emails", [])} - is_admin_user = email.lower() in admin_emails + is_admin_user = email.lower() in _get_admin_emails() if status == "closed" and not is_admin_user: return response(410, {"error": "Orders are closed for this week"}) if status == "not_found": diff --git a/src/server/generate_form.py b/src/server/generate_form.py index 7d9679e..26819d2 100644 --- a/src/server/generate_form.py +++ b/src/server/generate_form.py @@ -6,7 +6,7 @@ and submits orders to the backend (local Flask or cloud API Gateway). Usage: python3 generate_form.py # local mode - python3 generate_form.py --api-url URL --api-key KEY # cloud mode + python3 generate_form.py --api-url URL --google-client-id ID # cloud mode """ import argparse @@ -51,11 +51,14 @@ def generate_form( menu: dict, config: dict, api_url: str = "", - api_key: str = "", bulk_discount: float = 0, company_subsidy: float = 0, google_client_id: str = "", ) -> str: + google_client_id = google_client_id.strip() + if api_url and not google_client_id: + raise ValueError("Google client ID is required in cloud mode") + week = datetime.now().strftime("%Y-W%U") scraped_at = menu.get("scraped_at", "unknown") deadline = config.get("order_deadline", "Thursday 11:59 PM") @@ -79,7 +82,6 @@ def generate_form( "adminPdfUrl": ( f"{api_url}/api/admin/summary-pdf" if api_url else "/api/admin/summary-pdf" ), - "apiKey": api_key, "bulkDiscount": bulk_discount, "companySubsidy": company_subsidy, "googleClientId": google_client_id if use_google_auth else "", @@ -116,9 +118,6 @@ def main(): parser.add_argument( "--api-url", default="", help="API Gateway base URL (cloud mode)" ) - parser.add_argument( - "--api-key", default="", help="API key for order submission (cloud mode)" - ) parser.add_argument( "--bulk-discount", type=float, @@ -151,22 +150,28 @@ def main(): if args.company_subsidy is not None else config.get("company_subsidy_percent", 0) ) - google_client_id = args.google_client_id or config.get("google_client_id", "") - if not google_client_id: + google_client_id = ( + args.google_client_id or config.get("google_client_id", "") + ).strip() + if args.api_url and not google_client_id: try: import boto3 ssm = boto3.client("ssm") resp = ssm.get_parameter(Name="/meal-order-manager/google-client-id") - google_client_id = resp["Parameter"]["Value"] - except Exception: - pass + google_client_id = resp["Parameter"]["Value"].strip() + except Exception as exc: + parser.error( + "Google auth is required in cloud mode, but the Google client ID " + f"could not be loaded from SSM: {exc}" + ) + if args.api_url and not google_client_id: + parser.error("Google auth is required in cloud mode") html = generate_form( menu, config, api_url=args.api_url, - api_key=args.api_key, bulk_discount=bulk_discount, company_subsidy=company_subsidy, google_client_id=google_client_id, diff --git a/src/server/templates/form.js b/src/server/templates/form.js index 1788118..78456d2 100644 --- a/src/server/templates/form.js +++ b/src/server/templates/form.js @@ -7,7 +7,6 @@ const STATUS_URL = CONFIG.statusUrl; const ROSTER_URL = CONFIG.rosterUrl; const ADMIN_URL = CONFIG.adminUrl; const ADMIN_PDF_URL = CONFIG.adminPdfUrl; -const API_KEY = CONFIG.apiKey; const WEEK = CONFIG.week; const BULK_DISCOUNT = CONFIG.bulkDiscount; const COMPANY_SUBSIDY = CONFIG.companySubsidy; @@ -428,11 +427,9 @@ async function submitOrder() { btn.textContent = 'Submitting...'; try { - const headers = { 'Content-Type': 'application/json' }; - if (API_KEY) headers['x-api-key'] = API_KEY; const res = await fetch(SUBMIT_URL, { method: 'POST', - headers, + headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(payload), }); const data = await res.json(); diff --git a/template.yaml b/template.yaml index 0ee8425..d51f97b 100644 --- a/template.yaml +++ b/template.yaml @@ -244,6 +244,10 @@ Resources: DefaultRouteSettings: ThrottlingBurstLimit: 50 ThrottlingRateLimit: 100 + RouteSettings: + 'POST /api/submit-order': + ThrottlingBurstLimit: 10 + ThrottlingRateLimit: 5 # CORS only allows the production domain. For local development, use the # Flask dev server (app.py) which proxies API requests and doesn't enforce CORS. CorsConfiguration: @@ -260,7 +264,6 @@ Resources: - OPTIONS AllowHeaders: - Content-Type - - x-api-key - Authorization MaxAge: 3600 @@ -276,7 +279,6 @@ Resources: Timeout: 10 Environment: Variables: - 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 @@ -284,9 +286,6 @@ Resources: - DynamoDBCrudPolicy: TableName: !Ref OrdersTable - Statement: - - Effect: Allow - Action: secretsmanager:GetSecretValue - Resource: !Sub 'arn:aws:secretsmanager:${AWS::Region}:${AWS::AccountId}:secret:meal-order-manager/*' - Effect: Allow Action: lambda:InvokeFunction Resource: !GetAtt SlackNotifierFunction.Arn diff --git a/tests/test_generate_form.py b/tests/test_generate_form.py index 056b3f3..fe6e2b2 100644 --- a/tests/test_generate_form.py +++ b/tests/test_generate_form.py @@ -7,6 +7,7 @@ import json import re import sys from pathlib import Path +from types import SimpleNamespace import pytest @@ -16,7 +17,9 @@ FIXTURES = Path(__file__).resolve().parent / "fixtures" sys.path.insert(0, str(SERVER_DIR)) -from generate_form import generate_form # noqa: E402 +import generate_form as generate_form_module # noqa: E402 + +generate_form = generate_form_module.generate_form EXPECTED_CONFIG_KEYS = { "authMode", @@ -28,7 +31,6 @@ EXPECTED_CONFIG_KEYS = { "rosterUrl", "adminUrl", "adminPdfUrl", - "apiKey", "bulkDiscount", "companySubsidy", "googleClientId", @@ -54,19 +56,25 @@ def _extract_config(html: str) -> dict: return json.loads(match.group(1)) -def _render(*, google: bool = False, api_key: str = "test-key") -> str: +def _render(*, google: bool = False) -> str: menu, config = _load_fixtures() return generate_form( menu, config, - api_url="https://api.example.com", - api_key=api_key, + api_url="https://api.example.com" if google else "", bulk_discount=10, company_subsidy=50, google_client_id="google-client.apps.googleusercontent.com" if google else "", ) +def _render_for_browser(*, google: bool = False) -> str: + html = _render(google=google) + if not google: + html = html.replace("", '', 1) + return html + + def _google_credential() -> str: payload = json.dumps( { @@ -112,6 +120,64 @@ def _mock_form_routes(page): class TestGenerateFormStructural: + def test_cloud_mode_requires_google_auth(self): + menu, config = _load_fixtures() + for client_id in ("", " "): + with pytest.raises(ValueError, match="Google client ID is required"): + generate_form( + menu, + config, + api_url="https://api.example.com", + google_client_id=client_id, + ) + + def test_cloud_cli_rejects_whitespace_ssm_client_id(self, monkeypatch, capsys): + menu, config = _load_fixtures() + ssm = SimpleNamespace(get_parameter=lambda **_: {"Parameter": {"Value": " "}}) + monkeypatch.setitem(sys.modules, "boto3", SimpleNamespace(client=lambda _: ssm)) + monkeypatch.setattr(generate_form_module, "load_config", lambda: config) + monkeypatch.setattr(generate_form_module, "latest_menu", lambda: menu) + monkeypatch.setattr( + sys, + "argv", + ["generate_form.py", "--api-url", "https://api.example.com"], + ) + + with pytest.raises(SystemExit) as exc_info: + generate_form_module.main() + + assert exc_info.value.code == 2 + assert "Google auth is required in cloud mode" in capsys.readouterr().err + + def test_generated_form_contains_no_api_key_handling(self): + html = _render(google=True) + assert "apiKey" not in _extract_config(html) + assert "API_KEY" not in html + assert "x-api-key" not in html + + def test_cloud_configuration_has_no_form_api_key(self): + template = (REPO_ROOT / "template.yaml").read_text() + workflow = (REPO_ROOT / ".github" / "workflows" / "weekly-menu.yml").read_text() + readme = (REPO_ROOT / "README.md").read_text() + for text in (template, workflow): + assert "FORM_APIKEY" not in text + assert "form-api-key" not in text + assert "x-api-key" not in text + assert "--api-key" not in text + assert ( + "`meal-order-manager/form-api-key` secret remains until this change " + "is deployed and verified" + ) in readme + + def test_submit_route_has_explicit_throttling(self): + template = (REPO_ROOT / "template.yaml").read_text() + assert "'POST /api/submit-order':" in template + submit_settings = template.split("'POST /api/submit-order':", 1)[1].split( + "CorsConfiguration:", 1 + )[0] + assert "ThrottlingBurstLimit: 10" in submit_settings + assert "ThrottlingRateLimit: 5" in submit_settings + def test_local_and_google_render(self): local = _render(google=False) google = _render(google=True) @@ -291,7 +357,7 @@ class TestGenerateFormPlaywright: sync_api = pytest.importorskip("playwright.sync_api") sync_playwright = sync_api.sync_playwright - html = _render(google=False, api_key="") + html = _render_for_browser(google=False) # Stub status/roster so the page does not hang on network; roster fallback # in loadRoster still applies if fetch fails — intercept to be deterministic. out = tmp_path_factory.mktemp("form") / "order-form.html" @@ -321,7 +387,7 @@ class TestGenerateFormPlaywright: sync_playwright = sync_api.sync_playwright html_path = tmp_path / f"order-form-{'google' if google else 'local'}.html" - html_path.write_text(_render(google=google, api_key="")) + html_path.write_text(_render_for_browser(google=google)) with sync_playwright() as p: try: @@ -600,7 +666,7 @@ class TestGenerateFormPlaywright: @pytest.fixture def mobile_admin_page(self, tmp_path, browser_page): - html = _render(google=False, api_key="") + html = _render_for_browser(google=False) out = tmp_path / "admin-order-form.html" out.write_text(html) hostile_name = """O'Reilly "Ops\"""" @@ -763,7 +829,7 @@ class TestGenerateFormPlaywright: page.locator('.admin-order-card [data-admin-action="save-edit"]').click() page.wait_for_function( "() => document.querySelector('.admin-order-card')" - '.textContent.includes("Luis\' Lomo Saltado ×2")' # noqa: RUF001 + '?.textContent.includes("Luis\' Lomo Saltado ×2")' # noqa: RUF001 ) assert state["orders"][0]["employee_name"] == hostile_name assert state["orders"][0]["total"] == 41.0 @@ -845,7 +911,7 @@ class TestGenerateFormGooglePlaywright: ) from exc out = tmp_path_factory.mktemp("google-form") / "order-form.html" - out.write_text(_render(google=True, api_key="")) + out.write_text(_render_for_browser(google=True)) with sync_playwright() as p: try: diff --git a/tests/test_submit_order.py b/tests/test_submit_order.py index 47a9037..a4ee2b7 100644 --- a/tests/test_submit_order.py +++ b/tests/test_submit_order.py @@ -1,6 +1,6 @@ """Unit tests for functions/submit_order/handler.py -All external dependencies (DynamoDB, SSM, Secrets Manager, Lambda invoke) are +All external dependencies (DynamoDB, SSM, Google tokeninfo, Lambda invoke) are mocked — no real AWS calls are made. """ @@ -18,7 +18,6 @@ import pytest # Environment variables required by the handler at import time # --------------------------------------------------------------------------- os.environ.setdefault("TABLE_NAME", "test-orders-table") -os.environ.setdefault("FORM_APIKEY_SM_NAME", "test/form-api-key") os.environ.setdefault( "SLACK_NOTIFIER_ARN", "arn:aws:lambda:us-east-1:000000000000:function:test-slack-notifier", @@ -42,8 +41,7 @@ _spec.loader.exec_module(submit_order_handler) # Helpers # --------------------------------------------------------------------------- EASTERN = ZoneInfo("America/New_York") -TEST_API_KEY = "test-api-key-12345" -VALID_GOOGLE_CLIENT_ID = "123456789.apps.googleusercontent.com" +VALID_GOOGLE_CLIENT_ID = "test-google-client-id" def _make_event( @@ -71,9 +69,9 @@ def _make_event( def _submit_event( items, - api_key=TEST_API_KEY, employee_name="Test User", employee_email="test.user@seahavenind.com", + google_token="valid-google-token", extra_body=None, ): """Shortcut for a typical POST /submit event with items.""" @@ -82,13 +80,14 @@ def _submit_event( "employee_email": employee_email, "items": items, } + if google_token is not None: + body["google_id_token"] = google_token if extra_body: body.update(extra_body) return _make_event( method="POST", path="/submit", body=body, - headers={"x-api-key": api_key}, ) @@ -129,13 +128,12 @@ def _menu_doc_from_retail_pairs(pairs): # --------------------------------------------------------------------------- # We need to reset module-level caches between tests to avoid cross-test -# leakage. The handler module caches _api_key, _settings, _google_client_id. +# leakage. The handler module caches settings and the Google client ID. @pytest.fixture(autouse=True) def _reset_handler_caches(): """Reset handler module-level caches before each test.""" - submit_order_handler._api_key = None submit_order_handler._settings = None submit_order_handler._settings_ts = 0.0 submit_order_handler._google_client_id = None @@ -153,6 +151,24 @@ def _reset_shared_caches(): yield +@pytest.fixture(autouse=True) +def _mock_google_tokeninfo(): + """Return a valid company identity unless a test overrides tokeninfo.""" + mock_resp = MagicMock() + mock_resp.read.return_value = json.dumps( + { + "aud": VALID_GOOGLE_CLIENT_ID, + "hd": "seahavenind.com", + "name": "Test User", + "email": "test.user@seahavenind.com", + } + ).encode() + mock_resp.__enter__ = MagicMock(return_value=mock_resp) + mock_resp.__exit__ = MagicMock(return_value=False) + with patch("submit_order_handler.urllib.request.urlopen", return_value=mock_resp): + yield + + # =========================================================================== # PRICING PIPELINE (Critical) # =========================================================================== @@ -166,13 +182,13 @@ def _reset_shared_caches(): "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 10, "company_subsidy_percent": 50}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) @patch("submit_order_handler.get_menu") def test_discount_two_step_rounding( mock_get_menu, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -217,13 +233,13 @@ def test_discount_two_step_rounding( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 50, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) @patch("submit_order_handler.get_menu") def test_discount_rounding_half_up_boundary( mock_get_menu, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -263,13 +279,13 @@ def test_discount_rounding_half_up_boundary( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": -5, "company_subsidy_percent": 150}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) @patch("submit_order_handler.get_menu") def test_discount_clamping( mock_get_menu, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -309,13 +325,13 @@ def test_discount_clamping( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) @patch("submit_order_handler.get_menu") def test_discount_both_zero( mock_get_menu, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -358,13 +374,13 @@ def test_discount_both_zero( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 10, "company_subsidy_percent": 25}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) @patch("submit_order_handler.get_menu") def test_total_summation_multiple_items( mock_get_menu, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -417,13 +433,11 @@ def test_total_summation_multiple_items( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch("submit_order_handler.get_parameter") @patch("submit_order_handler.get_menu") def test_missing_ssm_param_with_env_var_fails_closed( mock_get_menu, mock_get_parameter, - mock_secret, mock_settings, mock_week, mock_status, @@ -460,7 +474,6 @@ def test_missing_ssm_param_with_env_var_fails_closed( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID ) @@ -468,7 +481,6 @@ def test_missing_ssm_param_with_env_var_fails_closed( def test_google_auth_required_when_configured( mock_gac, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -480,7 +492,7 @@ def test_google_auth_required_when_configured( items = _make_items([(10.00, 1)]) # Body has name/email but no google_id_token - event = _submit_event(items) + event = _submit_event(items, google_token=None) result = lambda_handler(event, None) status, body = _parse_response(result) @@ -499,7 +511,6 @@ def test_google_auth_required_when_configured( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID ) @@ -507,7 +518,6 @@ def test_google_auth_required_when_configured( def test_google_auth_bypass_prevention( mock_gac, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -527,7 +537,6 @@ def test_google_auth_bypass_prevention( method="POST", path="/submit", body=body, - headers={"x-api-key": TEST_API_KEY}, ) result = lambda_handler(event, None) status, body_resp = _parse_response(result) @@ -544,7 +553,6 @@ def test_google_auth_bypass_prevention( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID ) @@ -554,7 +562,6 @@ def test_google_token_audience_mismatch( mock_gac, mock_urlopen, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -595,7 +602,6 @@ def test_google_token_audience_mismatch( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID ) @@ -605,7 +611,6 @@ def test_google_token_domain_mismatch( mock_gac, mock_urlopen, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -645,7 +650,6 @@ def test_google_token_domain_mismatch( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID ) @@ -658,7 +662,6 @@ def test_google_token_service_unavailable( mock_gac, mock_urlopen, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -688,7 +691,6 @@ def test_google_token_service_unavailable( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID ) @@ -707,7 +709,6 @@ def test_google_token_http_error_returns_403( mock_gac, mock_urlopen, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -739,7 +740,6 @@ def test_google_token_http_error_returns_403( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", side_effect=Exception("ParameterNotFound"), @@ -748,7 +748,6 @@ def test_google_token_http_error_returns_403( def test_ssm_failure_fails_closed( mock_gac, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -780,7 +779,6 @@ def test_ssm_failure_fails_closed( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID ) @@ -792,7 +790,6 @@ def test_google_token_valid_seahaven_com_domain( mock_gac, mock_urlopen, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -837,7 +834,6 @@ def test_google_token_valid_seahaven_com_domain( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID ) @@ -849,7 +845,6 @@ def test_google_token_valid_success( mock_gac, mock_urlopen, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -903,20 +898,18 @@ def test_google_token_valid_success( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch("submit_order_handler._get_google_client_id", return_value="") @patch("submit_order_handler.get_menu") -def test_manual_fallback_when_google_not_configured( +def test_missing_google_client_id_fails_closed( mock_get_menu, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam, ): - """No Google client ID configured -> manual name/email accepted and order saved.""" + """No Google client ID configured -> 503, never manual fallback.""" from submit_order_handler import lambda_handler items = _make_items([(12.00, 1)]) @@ -927,15 +920,8 @@ def test_manual_fallback_when_google_not_configured( result = lambda_handler(event, None) status, body = _parse_response(result) - assert status == 200, f"Expected 200, got {status}: {body}" - - saved_order = mock_put.call_args[0][2] - assert saved_order["employee_name"] == "Manual User", ( - f"Name should be from manual input, got: {saved_order['employee_name']}" - ) - assert saved_order["employee_email"] == "manual@seahavenind.com", ( - f"Email should be from manual input, got: {saved_order['employee_email']}" - ) + assert status == 503, f"Expected fail-closed 503, got {status}: {body}" + mock_put.assert_not_called() @patch("submit_order_handler._lambda") @@ -946,24 +932,30 @@ def test_manual_fallback_when_google_not_configured( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") -def test_submit_invalid_api_key( - mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) +@patch("submit_order_handler.get_menu") +def test_submit_requires_no_api_key( + mock_get_menu, + mock_gcid, + mock_settings, + mock_week, + mock_status, + mock_put, + mock_lam, ): - """Wrong x-api-key -> 403.""" + """A Google-authenticated submission succeeds without a shared API key.""" from submit_order_handler import lambda_handler items = _make_items([(10.00, 1)]) - event = _submit_event(items, api_key="wrong-api-key") + mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.00, 1)]) + event = _submit_event(items) result = lambda_handler(event, None) status, body = _parse_response(result) - assert status == 403, f"Expected 403 for invalid API key, got {status}: {body}" - assert "Invalid API key" in body["error"], ( - f"Expected 'Invalid API key' in error, got: {body['error']}" - ) - mock_put.assert_not_called() + assert status == 200, f"Expected 200 without API key, got {status}: {body}" + mock_put.assert_called_once() # =========================================================================== @@ -979,13 +971,21 @@ def test_submit_invalid_api_key( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) +@patch( + "submit_order_handler._verify_google_token", + return_value=( + {"name": "Adam Moussa", "email": "Adam.Moussa@seahavenind.com"}, + "ok", + ), +) @patch("submit_order_handler.get_menu") def test_slug_from_email( mock_get_menu, + mock_verify, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -1020,33 +1020,46 @@ def test_slug_from_email( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) +@patch("submit_order_handler._verify_google_token") @patch("submit_order_handler.get_menu") def test_slug_edge_cases( mock_get_menu, + mock_verify, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam, ): - """Slug uses full lowercase email, preserving uniqueness across domains.""" + """Slug uses the full lowercase Google email across allowed domains.""" from submit_order_handler import lambda_handler + mock_verify.side_effect = [ + ( + { + "name": "First Middle Last", + "email": "First.Middle.Last@seahavenind.com", + }, + "ok", + ), + ({"name": "Bob Smith", "email": "Bob@seahaven.com"}, "ok"), + ] + # Test 1: full email preserved items = _make_items([(10.00, 1)]) mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.00, 1)]) event = _submit_event( items, employee_name="First Middle Last", - employee_email="First.Middle.Last@x.com", + employee_email="ignored@seahavenind.com", ) lambda_handler(event, None) slug_1 = mock_put.call_args[0][1] - assert slug_1 == "first.middle.last@x.com", ( + assert slug_1 == "first.middle.last@seahavenind.com", ( f"Slug should be full lowercase email, got '{slug_1}'" ) @@ -1054,11 +1067,11 @@ def test_slug_edge_cases( mock_put.reset_mock() mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.00, 1)]) event = _submit_event( - items, employee_name="Bob Smith", employee_email="Bob@other.com" + items, employee_name="Ignored", employee_email="ignored@seahavenind.com" ) lambda_handler(event, None) slug_2 = mock_put.call_args[0][1] - assert slug_2 == "bob@other.com", ( + assert slug_2 == "bob@seahaven.com", ( f"Slug should be full lowercase email, got '{slug_2}'" ) assert slug_1 != slug_2, "Different emails must produce different slugs" @@ -1191,12 +1204,17 @@ def test_form_status_closed_saturday_before_dst_end(mock_week, mock_status): "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) +@patch( + "submit_order_handler._verify_google_token", + return_value=({"name": "", "email": "test@seahavenind.com"}, "ok"), +) def test_submit_missing_name( - mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam + mock_verify, mock_gcid, mock_settings, mock_week, mock_status, mock_put, mock_lam ): - """Empty employee name -> 400.""" + """A Google identity without a name -> 400.""" from submit_order_handler import lambda_handler items = _make_items([(10.00, 1)]) @@ -1221,12 +1239,17 @@ def test_submit_missing_name( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) +@patch( + "submit_order_handler._verify_google_token", + return_value=({"name": "Test User", "email": ""}, "ok"), +) def test_submit_missing_email( - mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam + mock_verify, mock_gcid, mock_settings, mock_week, mock_status, mock_put, mock_lam ): - """Empty employee email -> 400.""" + """A Google identity without an email -> 400.""" from submit_order_handler import lambda_handler items = _make_items([(10.00, 1)]) @@ -1249,10 +1272,11 @@ def test_submit_missing_email( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) def test_submit_zero_quantity_only( - mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam + mock_gcid, mock_settings, mock_week, mock_status, mock_put, mock_lam ): """All items with quantity 0 -> 400.""" from submit_order_handler import lambda_handler @@ -1277,10 +1301,11 @@ def test_submit_zero_quantity_only( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) def test_submit_form_closed( - mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam + mock_gcid, mock_settings, mock_week, mock_status, mock_put, mock_lam ): """Form closed -> 410.""" from submit_order_handler import lambda_handler @@ -1309,7 +1334,6 @@ def test_submit_form_closed( "admin_emails": ["adam@seahavenind.com"], }, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID ) @@ -1321,7 +1345,6 @@ def test_admin_can_submit_when_form_closed( mock_gac, mock_urlopen, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -1366,7 +1389,6 @@ def test_admin_can_submit_when_form_closed( "admin_emails": ["adam@seahavenind.com"], }, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) @patch( "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID ) @@ -1378,7 +1400,6 @@ def test_non_admin_blocked_when_form_closed( mock_gac, mock_urlopen, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -1419,10 +1440,11 @@ def test_non_admin_blocked_when_form_closed( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) def test_submit_no_menu( - mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam + mock_gcid, mock_settings, mock_week, mock_status, mock_put, mock_lam ): """No menu available -> 404.""" from submit_order_handler import lambda_handler @@ -1447,13 +1469,13 @@ def test_submit_no_menu( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) @patch("submit_order_handler.get_menu") def test_unknown_meal_returns_400( mock_get_menu, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -1483,13 +1505,13 @@ def test_unknown_meal_returns_400( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) @patch("submit_order_handler.get_menu") def test_retail_price_from_menu_not_request_body( mock_get_menu, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -1519,13 +1541,13 @@ def test_retail_price_from_menu_not_request_body( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) @patch("submit_order_handler.get_menu") def test_menu_missing_priced_meals_returns_503( mock_get_menu, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status, @@ -1557,13 +1579,13 @@ def test_menu_missing_priced_meals_returns_503( "submit_order_handler.get_settings", return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0}, ) -@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY) -@patch("submit_order_handler._get_google_client_id", return_value="") +@patch( + "submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID +) @patch("submit_order_handler.get_menu") def test_slack_failure_does_not_fail_order( mock_get_menu, mock_gcid, - mock_secret, mock_settings, mock_week, mock_status,