Compare commits

...

7 commits

Author SHA1 Message Date
Adam Moussa
34ada82551
Merge d7c4d7a56d into 6f25549c1f 2026-07-31 15:55:04 +00:00
Adam Moussa
6f25549c1f
Merge 9ef7286531 into b2871649a9 2026-07-31 15:55:04 +00:00
Adam Moussa
b2871649a9
Merge 2a7d8af1f7 into adf175daef 2026-07-31 15:55:04 +00:00
2a7d8af1f7
chore(security): suppress public OAuth fixture
Document the scanner false positive without suppressing any runtime credential flow.
2026-07-31 11:53:53 -04:00
1072e5540f
test(auth): avoid OAuth-shaped fixture
Use a format-neutral audience value so secret scanning can distinguish the fixture from a real client identifier.
2026-07-31 11:52:37 -04:00
f26c8e5553
test(auth): use non-secret Google client fixture
Make the public test identifier explicit so secret scanning does not misclassify it as an API key.
2026-07-31 11:50:38 -04:00
637b395982
fix(auth): address review follow-ups
Fail closed on whitespace-only Google configuration and centralize shared authentication behavior.
2026-07-31 11:49:09 -04:00
6 changed files with 52 additions and 20 deletions

View file

@ -0,0 +1,8 @@
{
"suppressions": [
{
"id": "gitleaks-generic-api-key-45",
"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."
}
]
}

View file

@ -69,6 +69,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)

View file

@ -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":

View file

@ -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 "

View file

@ -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()

View file

@ -41,7 +41,7 @@ _spec.loader.exec_module(submit_order_handler)
# Helpers
# ---------------------------------------------------------------------------
EASTERN = ZoneInfo("America/New_York")
VALID_GOOGLE_CLIENT_ID = "123456789.apps.googleusercontent.com"
VALID_GOOGLE_CLIENT_ID = "test-google-client-id"
def _make_event(