mirror of
https://github.com/Sea-Haven-Industries/meal-order-manager.git
synced 2026-10-04 19:32:00 +00:00
Fix close-form weekday guard and SSM auth fail-open
- Close form guard: check weekday == 3 (Thursday), not 4 (Friday) — the crons fire at Thursday 11:59 PM ET, when weekday() is 3 - SSM fail-closed: separate _google_auth_configured() (checks env var) from _get_google_client_id() (fetches value). If auth is configured but the SSM fetch fails, return 503 instead of silently falling back to manual auth - Update close_form tests to use Thursday dates - Add test_ssm_failure_fails_closed
This commit is contained in:
parent
5c040bf55c
commit
5db992b0be
4 changed files with 114 additions and 37 deletions
|
|
@ -13,10 +13,10 @@ EASTERN = ZoneInfo("America/New_York")
|
||||||
|
|
||||||
def lambda_handler(event, context):
|
def lambda_handler(event, context):
|
||||||
now_et = datetime.now(EASTERN)
|
now_et = datetime.now(EASTERN)
|
||||||
if not (now_et.weekday() == 4 and now_et.hour >= 23):
|
if not (now_et.weekday() == 3 and now_et.hour >= 23):
|
||||||
return {
|
return {
|
||||||
"status": "skipped",
|
"status": "skipped",
|
||||||
"reason": "outside close window (must be Friday >= 11 PM ET)",
|
"reason": "outside close window (must be Thursday >= 11 PM ET)",
|
||||||
}
|
}
|
||||||
|
|
||||||
week = event.get("week", current_week())
|
week = event.get("week", current_week())
|
||||||
|
|
|
||||||
|
|
@ -50,19 +50,17 @@ def _get_discount_settings() -> tuple[Decimal, Decimal]:
|
||||||
return _settings
|
return _settings
|
||||||
|
|
||||||
|
|
||||||
|
def _google_auth_configured() -> bool:
|
||||||
|
return bool(os.environ.get("GOOGLE_CLIENT_ID_PARAM", ""))
|
||||||
|
|
||||||
|
|
||||||
def _get_google_client_id() -> str:
|
def _get_google_client_id() -> str:
|
||||||
global _google_client_id, _google_client_id_ts
|
global _google_client_id, _google_client_id_ts
|
||||||
now = time.monotonic()
|
now = time.monotonic()
|
||||||
if _google_client_id is None or (now - _google_client_id_ts) > CACHE_TTL_SECONDS:
|
if _google_client_id is None or (now - _google_client_id_ts) > CACHE_TTL_SECONDS:
|
||||||
param = os.environ.get("GOOGLE_CLIENT_ID_PARAM", "")
|
param = os.environ.get("GOOGLE_CLIENT_ID_PARAM", "")
|
||||||
if param:
|
if param:
|
||||||
try:
|
_google_client_id = get_parameter(param, decrypt=False) or ""
|
||||||
_google_client_id = get_parameter(param, decrypt=False) or ""
|
|
||||||
except Exception as exc:
|
|
||||||
logger.warning(
|
|
||||||
"Could not fetch Google client ID from SSM (%s): %s", param, exc
|
|
||||||
)
|
|
||||||
_google_client_id = ""
|
|
||||||
else:
|
else:
|
||||||
_google_client_id = ""
|
_google_client_id = ""
|
||||||
_google_client_id_ts = now
|
_google_client_id_ts = now
|
||||||
|
|
@ -160,12 +158,24 @@ def handle_submit(event):
|
||||||
return response(400, {"error": "Invalid JSON"})
|
return response(400, {"error": "Invalid JSON"})
|
||||||
|
|
||||||
# --- Authentication ---
|
# --- Authentication ---
|
||||||
# If Google auth is configured, require a valid google_id_token.
|
# If Google auth is configured (SSM param name is set), require a valid
|
||||||
# Manual name/email fallback is only allowed when Google auth is NOT configured.
|
# google_id_token. Manual fallback is only allowed when auth is NOT configured.
|
||||||
google_auth_enabled = bool(_get_google_client_id())
|
# If SSM fetch fails, fail closed (503) rather than silently disabling auth.
|
||||||
google_token = body.get("google_id_token")
|
google_token = body.get("google_id_token")
|
||||||
|
|
||||||
if google_auth_enabled:
|
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:
|
if not google_token:
|
||||||
return response(403, {"error": "Google authentication is required"})
|
return response(403, {"error": "Google authentication is required"})
|
||||||
user_info, verify_status = _verify_google_token(google_token)
|
user_info, verify_status = _verify_google_token(google_token)
|
||||||
|
|
|
||||||
|
|
@ -32,33 +32,42 @@ def _make_datetime(year, month, day, hour, minute=0):
|
||||||
|
|
||||||
class TestCloseFormGuard:
|
class TestCloseFormGuard:
|
||||||
@patch("close_form_handler.datetime")
|
@patch("close_form_handler.datetime")
|
||||||
def test_skipped_on_thursday(self, mock_dt):
|
def test_skipped_on_wednesday(self, mock_dt):
|
||||||
"""Thursday 11pm ET -> skipped (not Friday)."""
|
"""Wednesday 11pm ET -> skipped (not Thursday)."""
|
||||||
mock_dt.now.return_value = _make_datetime(2026, 5, 14, 23) # Thursday
|
mock_dt.now.return_value = _make_datetime(2026, 5, 13, 23) # Wednesday
|
||||||
|
|
||||||
result = close_form_handler.lambda_handler({}, None)
|
result = close_form_handler.lambda_handler({}, None)
|
||||||
|
|
||||||
assert result["status"] == "skipped"
|
assert result["status"] == "skipped"
|
||||||
|
|
||||||
@patch("close_form_handler.datetime")
|
@patch("close_form_handler.datetime")
|
||||||
def test_skipped_friday_before_11pm(self, mock_dt):
|
def test_skipped_on_friday(self, mock_dt):
|
||||||
"""Friday 3am ET (UTC cron fires but too early) -> skipped."""
|
"""Friday 3am ET (wrong-tz EDT cron during EST) -> skipped."""
|
||||||
mock_dt.now.return_value = _make_datetime(2026, 5, 15, 3) # Friday 3am
|
mock_dt.now.return_value = _make_datetime(2026, 5, 15, 3) # Friday 3am
|
||||||
|
|
||||||
result = close_form_handler.lambda_handler({}, None)
|
result = close_form_handler.lambda_handler({}, None)
|
||||||
|
|
||||||
assert result["status"] == "skipped"
|
assert result["status"] == "skipped"
|
||||||
|
|
||||||
|
@patch("close_form_handler.datetime")
|
||||||
|
def test_skipped_thursday_before_11pm(self, mock_dt):
|
||||||
|
"""Thursday 10pm ET -> skipped (too early)."""
|
||||||
|
mock_dt.now.return_value = _make_datetime(2026, 5, 14, 22) # Thursday 10pm
|
||||||
|
|
||||||
|
result = close_form_handler.lambda_handler({}, None)
|
||||||
|
|
||||||
|
assert result["status"] == "skipped"
|
||||||
|
|
||||||
@patch("close_form_handler.set_form_status")
|
@patch("close_form_handler.set_form_status")
|
||||||
@patch("close_form_handler.get_form_status", return_value="open")
|
@patch("close_form_handler.get_form_status", return_value="open")
|
||||||
@patch("close_form_handler.current_week", return_value="2026-W19")
|
@patch("close_form_handler.current_week", return_value="2026-W19")
|
||||||
@patch("close_form_handler._lambda")
|
@patch("close_form_handler._lambda")
|
||||||
@patch("close_form_handler.datetime")
|
@patch("close_form_handler.datetime")
|
||||||
def test_runs_friday_at_11pm(
|
def test_runs_thursday_at_11pm(
|
||||||
self, mock_dt, mock_lam, mock_week, mock_status, mock_set
|
self, mock_dt, mock_lam, mock_week, mock_status, mock_set
|
||||||
):
|
):
|
||||||
"""Friday 11pm ET -> proceeds to close."""
|
"""Thursday 11pm ET -> proceeds to close."""
|
||||||
mock_dt.now.return_value = _make_datetime(2026, 5, 15, 23) # Friday 11pm
|
mock_dt.now.return_value = _make_datetime(2026, 5, 14, 23) # Thursday 11pm
|
||||||
|
|
||||||
result = close_form_handler.lambda_handler({}, None)
|
result = close_form_handler.lambda_handler({}, None)
|
||||||
|
|
||||||
|
|
@ -70,31 +79,22 @@ class TestCloseFormGuard:
|
||||||
@patch("close_form_handler.current_week", return_value="2026-W19")
|
@patch("close_form_handler.current_week", return_value="2026-W19")
|
||||||
@patch("close_form_handler._lambda")
|
@patch("close_form_handler._lambda")
|
||||||
@patch("close_form_handler.datetime")
|
@patch("close_form_handler.datetime")
|
||||||
def test_runs_friday_at_1159pm(
|
def test_runs_thursday_at_1159pm(
|
||||||
self, mock_dt, mock_lam, mock_week, mock_status, mock_set
|
self, mock_dt, mock_lam, mock_week, mock_status, mock_set
|
||||||
):
|
):
|
||||||
"""Friday 11:59pm ET -> proceeds to close."""
|
"""Thursday 11:59pm ET -> proceeds to close."""
|
||||||
mock_dt.now.return_value = _make_datetime(2026, 5, 15, 23, 59)
|
mock_dt.now.return_value = _make_datetime(2026, 5, 14, 23, 59)
|
||||||
|
|
||||||
result = close_form_handler.lambda_handler({}, None)
|
result = close_form_handler.lambda_handler({}, None)
|
||||||
|
|
||||||
assert result["status"] == "closed"
|
assert result["status"] == "closed"
|
||||||
|
|
||||||
@patch("close_form_handler.datetime")
|
|
||||||
def test_skipped_on_wednesday(self, mock_dt):
|
|
||||||
"""Wednesday at any hour -> skipped."""
|
|
||||||
mock_dt.now.return_value = _make_datetime(2026, 5, 13, 23)
|
|
||||||
|
|
||||||
result = close_form_handler.lambda_handler({}, None)
|
|
||||||
|
|
||||||
assert result["status"] == "skipped"
|
|
||||||
|
|
||||||
@patch("close_form_handler.get_form_status", return_value="closed")
|
@patch("close_form_handler.get_form_status", return_value="closed")
|
||||||
@patch("close_form_handler.current_week", return_value="2026-W19")
|
@patch("close_form_handler.current_week", return_value="2026-W19")
|
||||||
@patch("close_form_handler.datetime")
|
@patch("close_form_handler.datetime")
|
||||||
def test_already_closed(self, mock_dt, mock_week, mock_status):
|
def test_already_closed(self, mock_dt, mock_week, mock_status):
|
||||||
"""Friday 11pm but already closed -> returns already_closed."""
|
"""Thursday 11pm but already closed -> returns already_closed."""
|
||||||
mock_dt.now.return_value = _make_datetime(2026, 5, 15, 23)
|
mock_dt.now.return_value = _make_datetime(2026, 5, 14, 23)
|
||||||
|
|
||||||
result = close_form_handler.lambda_handler({}, None)
|
result = close_form_handler.lambda_handler({}, None)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -365,8 +365,16 @@ def test_total_summation_multiple_items(
|
||||||
@patch(
|
@patch(
|
||||||
"submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID
|
"submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID
|
||||||
)
|
)
|
||||||
|
@patch("submit_order_handler._google_auth_configured", return_value=True)
|
||||||
def test_google_auth_required_when_configured(
|
def test_google_auth_required_when_configured(
|
||||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
mock_gac,
|
||||||
|
mock_gcid,
|
||||||
|
mock_secret,
|
||||||
|
mock_settings,
|
||||||
|
mock_week,
|
||||||
|
mock_status,
|
||||||
|
mock_put,
|
||||||
|
mock_lam,
|
||||||
):
|
):
|
||||||
"""When Google auth is configured and no token is provided, return 403."""
|
"""When Google auth is configured and no token is provided, return 403."""
|
||||||
from submit_order_handler import lambda_handler
|
from submit_order_handler import lambda_handler
|
||||||
|
|
@ -396,8 +404,16 @@ def test_google_auth_required_when_configured(
|
||||||
@patch(
|
@patch(
|
||||||
"submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID
|
"submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID
|
||||||
)
|
)
|
||||||
|
@patch("submit_order_handler._google_auth_configured", return_value=True)
|
||||||
def test_google_auth_bypass_prevention(
|
def test_google_auth_bypass_prevention(
|
||||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
mock_gac,
|
||||||
|
mock_gcid,
|
||||||
|
mock_secret,
|
||||||
|
mock_settings,
|
||||||
|
mock_week,
|
||||||
|
mock_status,
|
||||||
|
mock_put,
|
||||||
|
mock_lam,
|
||||||
):
|
):
|
||||||
"""Google auth enabled + name/email in body but no token -> 403 (can't bypass)."""
|
"""Google auth enabled + name/email in body but no token -> 403 (can't bypass)."""
|
||||||
from submit_order_handler import lambda_handler
|
from submit_order_handler import lambda_handler
|
||||||
|
|
@ -434,7 +450,9 @@ def test_google_auth_bypass_prevention(
|
||||||
"submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID
|
"submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID
|
||||||
)
|
)
|
||||||
@patch("submit_order_handler.urllib.request.urlopen")
|
@patch("submit_order_handler.urllib.request.urlopen")
|
||||||
|
@patch("submit_order_handler._google_auth_configured", return_value=True)
|
||||||
def test_google_token_audience_mismatch(
|
def test_google_token_audience_mismatch(
|
||||||
|
mock_gac,
|
||||||
mock_urlopen,
|
mock_urlopen,
|
||||||
mock_gcid,
|
mock_gcid,
|
||||||
mock_secret,
|
mock_secret,
|
||||||
|
|
@ -483,7 +501,9 @@ def test_google_token_audience_mismatch(
|
||||||
"submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID
|
"submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID
|
||||||
)
|
)
|
||||||
@patch("submit_order_handler.urllib.request.urlopen")
|
@patch("submit_order_handler.urllib.request.urlopen")
|
||||||
|
@patch("submit_order_handler._google_auth_configured", return_value=True)
|
||||||
def test_google_token_domain_mismatch(
|
def test_google_token_domain_mismatch(
|
||||||
|
mock_gac,
|
||||||
mock_urlopen,
|
mock_urlopen,
|
||||||
mock_gcid,
|
mock_gcid,
|
||||||
mock_secret,
|
mock_secret,
|
||||||
|
|
@ -534,7 +554,9 @@ def test_google_token_domain_mismatch(
|
||||||
"submit_order_handler.urllib.request.urlopen",
|
"submit_order_handler.urllib.request.urlopen",
|
||||||
side_effect=urllib.error.URLError("Connection refused"),
|
side_effect=urllib.error.URLError("Connection refused"),
|
||||||
)
|
)
|
||||||
|
@patch("submit_order_handler._google_auth_configured", return_value=True)
|
||||||
def test_google_token_service_unavailable(
|
def test_google_token_service_unavailable(
|
||||||
|
mock_gac,
|
||||||
mock_urlopen,
|
mock_urlopen,
|
||||||
mock_gcid,
|
mock_gcid,
|
||||||
mock_secret,
|
mock_secret,
|
||||||
|
|
@ -581,7 +603,9 @@ def test_google_token_service_unavailable(
|
||||||
None,
|
None,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
@patch("submit_order_handler._google_auth_configured", return_value=True)
|
||||||
def test_google_token_http_error_returns_403(
|
def test_google_token_http_error_returns_403(
|
||||||
|
mock_gac,
|
||||||
mock_urlopen,
|
mock_urlopen,
|
||||||
mock_gcid,
|
mock_gcid,
|
||||||
mock_secret,
|
mock_secret,
|
||||||
|
|
@ -608,6 +632,47 @@ def test_google_token_http_error_returns_403(
|
||||||
mock_put.assert_not_called()
|
mock_put.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@patch("submit_order_handler._lambda")
|
||||||
|
@patch("submit_order_handler.put_order")
|
||||||
|
@patch("submit_order_handler.get_form_status", return_value="open")
|
||||||
|
@patch("submit_order_handler.current_week", return_value="2026-W20")
|
||||||
|
@patch(
|
||||||
|
"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"),
|
||||||
|
)
|
||||||
|
@patch("submit_order_handler._google_auth_configured", return_value=True)
|
||||||
|
def test_ssm_failure_fails_closed(
|
||||||
|
mock_gac,
|
||||||
|
mock_gcid,
|
||||||
|
mock_secret,
|
||||||
|
mock_settings,
|
||||||
|
mock_week,
|
||||||
|
mock_status,
|
||||||
|
mock_put,
|
||||||
|
mock_lam,
|
||||||
|
):
|
||||||
|
"""SSM fetch failure with auth configured -> 503, not silent fallback to manual."""
|
||||||
|
from submit_order_handler import lambda_handler
|
||||||
|
|
||||||
|
items = _make_items([(10.00, 1)])
|
||||||
|
event = _submit_event(items)
|
||||||
|
result = lambda_handler(event, None)
|
||||||
|
status, body = _parse_response(result)
|
||||||
|
|
||||||
|
assert status == 503, (
|
||||||
|
f"Expected 503 for SSM failure (fail-closed), got {status}: {body}"
|
||||||
|
)
|
||||||
|
assert "temporarily unavailable" in body["error"], (
|
||||||
|
f"Expected 'temporarily unavailable' in error, got: {body['error']}"
|
||||||
|
)
|
||||||
|
mock_put.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
@patch("submit_order_handler._lambda")
|
@patch("submit_order_handler._lambda")
|
||||||
@patch("submit_order_handler.put_order")
|
@patch("submit_order_handler.put_order")
|
||||||
@patch("submit_order_handler.get_form_status", return_value="open")
|
@patch("submit_order_handler.get_form_status", return_value="open")
|
||||||
|
|
@ -621,7 +686,9 @@ def test_google_token_http_error_returns_403(
|
||||||
"submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID
|
"submit_order_handler._get_google_client_id", return_value=VALID_GOOGLE_CLIENT_ID
|
||||||
)
|
)
|
||||||
@patch("submit_order_handler.urllib.request.urlopen")
|
@patch("submit_order_handler.urllib.request.urlopen")
|
||||||
|
@patch("submit_order_handler._google_auth_configured", return_value=True)
|
||||||
def test_google_token_valid_success(
|
def test_google_token_valid_success(
|
||||||
|
mock_gac,
|
||||||
mock_urlopen,
|
mock_urlopen,
|
||||||
mock_gcid,
|
mock_gcid,
|
||||||
mock_secret,
|
mock_secret,
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue