fix(auth): address review follow-ups

Fail closed on whitespace-only Google configuration and centralize shared authentication behavior.

README.md

@@ -60,6 +60,7 @@ Stack name: `meal-order-manager` (us-east-1)
 - **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
+  - 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)
 

functions/submit_order/handler.py

@@ -65,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", ""))
 
@@ -147,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():
@@ -163,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
@@ -422,21 +427,15 @@ def handle_submit(event):
         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"}
-        )
+        return _authentication_service_unavailable()
     if not client_id:
         logger.error("Google client ID is unavailable")
-        return response(
-            503, {"error": "Authentication service temporarily 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 response(
-            503, {"error": "Authentication service temporarily unavailable"}
-        )
+        return _authentication_service_unavailable()
     if user_info is None:
         return response(403, {"error": "Invalid or unauthorized Google account"})
     name = user_info["name"]
@@ -453,8 +452,7 @@ def handle_submit(event):
 
     week = current_week()
     status = get_form_status(week)
-    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":

src/server/generate_form.py

@@ -159,7 +159,7 @@ def main():
 
             ssm = boto3.client("ssm")
             resp = ssm.get_parameter(Name="/meal-order-manager/google-client-id")
-            google_client_id = resp["Parameter"]["Value"]
+            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 "

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",
@@ -128,6 +131,24 @@ class TestGenerateFormStructural:
                     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)
@@ -138,11 +159,15 @@ class TestGenerateFormStructural:
         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, readme):
+        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()