diff --git a/README.md b/README.md index facbca3..1cb48d6 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ sit in ALARM between runs). Alarm names follow `meal-order-manager-- Google Identity Services and portal Cognito ID tokens coexist until portal cutover. Google tokens use the tokeninfo endpoint. Portal tokens are verified locally against the configured Cognito issuer, audience, signature, expiry, token use, and email domain. Both paths accept only `seahavenind.com` and `seahaven.com` identities and fail closed when their SSM configuration is unavailable. The local Flask workflow can still use manual name and email entry when Google auth is not configured. -Set the `portal_cognito_issuer` and `portal_cognito_audience` HCP Terraform workspace variables from the matching internal-portal stage outputs. Switch both values together when moving from dev validation to the production portal pool. +Set the `portal_cognito_issuer` and `portal_cognito_audience` HCP Terraform workspace variables from one internal-portal stage. Add the other stage to `portal_cognito_extra_trust` so portal-dev and portal-prod tokens both work against this single meals API. Google federated portal users are accepted without an `email_verified=true` claim; identity still has to be a `seahavenind.com` or `seahaven.com` email from a trusted pool. ## Admin Panel diff --git a/functions/admin_authorizer/handler.py b/functions/admin_authorizer/handler.py index f406d76..cc93f30 100644 --- a/functions/admin_authorizer/handler.py +++ b/functions/admin_authorizer/handler.py @@ -96,6 +96,7 @@ def _verify_portal_token(token: str) -> dict | None: token, os.environ.get("PORTAL_COGNITO_ISSUER_PARAM", ""), os.environ.get("PORTAL_COGNITO_AUDIENCE_PARAM", ""), + os.environ.get("PORTAL_COGNITO_TRUST_PARAM", ""), ) diff --git a/functions/submit_order/handler.py b/functions/submit_order/handler.py index 27cf142..a006de3 100644 --- a/functions/submit_order/handler.py +++ b/functions/submit_order/handler.py @@ -174,6 +174,7 @@ def _verify_portal_token(token: str) -> dict | None: token, os.environ.get("PORTAL_COGNITO_ISSUER_PARAM", ""), os.environ.get("PORTAL_COGNITO_AUDIENCE_PARAM", ""), + os.environ.get("PORTAL_COGNITO_TRUST_PARAM", ""), ) diff --git a/src/shared/shared/cognito.py b/src/shared/shared/cognito.py index b4617f1..c0a7e2c 100644 --- a/src/shared/shared/cognito.py +++ b/src/shared/shared/cognito.py @@ -1,5 +1,6 @@ """Verification for portal Cognito ID tokens.""" +import json import logging import re from functools import lru_cache @@ -16,6 +17,11 @@ ALLOWED_EMAIL_DOMAINS = {"seahavenind.com", "seahaven.com"} _COGNITO_ISSUER_RE = re.compile( r"^https://cognito-idp\.[a-z0-9-]+\.amazonaws\.com/[A-Za-z0-9_-]+$" ) +_UNVERIFIED_DECODE = { + "verify_signature": False, + "verify_exp": False, + "verify_aud": False, +} class CognitoVerificationUnavailable(RuntimeError): @@ -27,18 +33,15 @@ def looks_like_cognito_token(token: str) -> bool: try: claims = jwt.decode( token, - options={ - "verify_signature": False, - "verify_exp": False, - "verify_aud": False, - }, + options=_UNVERIFIED_DECODE, + algorithms=["RS256"], ) except PyJWTError: return False issuer = claims.get("iss") return ( isinstance(issuer, str) - and bool(_COGNITO_ISSUER_RE.fullmatch(issuer)) + and bool(_COGNITO_ISSUER_RE.fullmatch(issuer.rstrip("/"))) and claims.get("token_use") == "id" ) @@ -54,23 +57,46 @@ def _jwk_client(issuer: str) -> PyJWKClient: def verify_cognito_id_token( - token: str, issuer_param: str, audience_param: str + token: str, + issuer_param: str, + audience_param: str, + trust_param: str = "", ) -> dict | None: """Verify a portal token and return its trusted identity claims.""" - if not token or not issuer_param or not audience_param: - logger.error("Cognito verification is not configured") + if not token: return None try: - issuer = get_parameter(issuer_param, decrypt=False).rstrip("/") - audience = get_parameter(audience_param, decrypt=False) + trusted = _trusted_clients(issuer_param, audience_param, trust_param) + except CognitoVerificationUnavailable: + raise except Exception as exc: logger.error("Failed to load Cognito verification parameters: %s", exc) raise CognitoVerificationUnavailable from exc - if not _COGNITO_ISSUER_RE.fullmatch(issuer) or not audience: - logger.error("Cognito verification parameters are invalid") - raise CognitoVerificationUnavailable + if not trusted: + logger.error("Cognito verification is not configured") + return None + + try: + unverified = jwt.decode( + token, + options=_UNVERIFIED_DECODE, + algorithms=["RS256"], + ) + except PyJWTError as exc: + logger.warning("Cognito token rejected: %s", type(exc).__name__) + return None + + issuer = unverified.get("iss") + if not isinstance(issuer, str): + logger.warning("Cognito token rejected: missing issuer") + return None + issuer = issuer.rstrip("/") + audience = trusted.get(issuer) + if not audience: + logger.warning("Cognito token rejected: untrusted issuer") + return None try: signing_key = _jwk_client(issuer).get_signing_key_from_jwt(token) @@ -84,11 +110,9 @@ def verify_cognito_id_token( "require": [ "aud", "email", - "email_verified", "exp", "iat", "iss", - "name", "token_use", ] }, @@ -103,17 +127,56 @@ def verify_cognito_id_token( logger.warning("Cognito token rejected: %s", type(exc).__name__) return None - email = claims.get("email") - name = claims.get("name") + return _identity_from_claims(claims) + + +def _trusted_clients( + issuer_param: str, audience_param: str, trust_param: str +) -> dict[str, str]: + trusted: dict[str, str] = {} + if trust_param: + raw = get_parameter(trust_param, decrypt=False) + items = json.loads(raw) + if not isinstance(items, list): + raise CognitoVerificationUnavailable + for item in items: + if not isinstance(item, dict): + continue + issuer = str(item.get("issuer", "")).rstrip("/") + audience = str(item.get("audience", "")).strip() + if _COGNITO_ISSUER_RE.fullmatch(issuer) and audience: + trusted[issuer] = audience + if issuer_param and audience_param: + issuer = get_parameter(issuer_param, decrypt=False).rstrip("/") + audience = get_parameter(audience_param, decrypt=False).strip() + if not _COGNITO_ISSUER_RE.fullmatch(issuer) or not audience: + if not trusted: + logger.error("Cognito verification parameters are invalid") + raise CognitoVerificationUnavailable + else: + trusted.setdefault(issuer, audience) + return trusted + + +def _identity_from_claims(claims: dict) -> dict | None: if claims.get("token_use") != "id": return None - if claims.get("email_verified") is not True: - return None - if not isinstance(email, str) or not isinstance(name, str): - return None - if not email.strip() or not name.strip() or "@" not in email: + email = claims.get("email") + if not isinstance(email, str) or not email.strip() or "@" not in email: return None + email = email.strip() if email.rsplit("@", 1)[1].lower() not in ALLOWED_EMAIL_DOMAINS: return None + name = _display_name(claims, email) + if not name: + return None + return {"name": name, "email": email} - return {"name": name.strip(), "email": email.strip()} + +def _display_name(claims: dict, email: str) -> str | None: + for key in ("name", "given_name"): + value = claims.get(key) + if isinstance(value, str) and value.strip(): + return value.strip() + local = email.split("@", 1)[0].strip() + return local or None diff --git a/terraform/lambda.tf b/terraform/lambda.tf index 43347bf..077c0ba 100644 --- a/terraform/lambda.tf +++ b/terraform/lambda.tf @@ -41,6 +41,7 @@ resource "aws_lambda_function" "submit_order" { GOOGLE_CLIENT_ID_PARAM = local.google_client_id_param PORTAL_COGNITO_ISSUER_PARAM = aws_ssm_parameter.portal_cognito_issuer.name PORTAL_COGNITO_AUDIENCE_PARAM = aws_ssm_parameter.portal_cognito_audience.name + PORTAL_COGNITO_TRUST_PARAM = aws_ssm_parameter.portal_cognito_trust.name }) } @@ -75,6 +76,7 @@ resource "aws_lambda_function" "admin_authorizer" { GOOGLE_CLIENT_ID_PARAM = local.google_client_id_param PORTAL_COGNITO_ISSUER_PARAM = aws_ssm_parameter.portal_cognito_issuer.name PORTAL_COGNITO_AUDIENCE_PARAM = aws_ssm_parameter.portal_cognito_audience.name + PORTAL_COGNITO_TRUST_PARAM = aws_ssm_parameter.portal_cognito_trust.name }) } diff --git a/terraform/locals.tf b/terraform/locals.tf index f9e9567..4ddaa1f 100644 --- a/terraform/locals.tf +++ b/terraform/locals.tf @@ -24,6 +24,7 @@ locals { portal_cognito_issuer_param = "${local.ssm_prefix}/portal-cognito-issuer" portal_cognito_audience_param = "${local.ssm_prefix}/portal-cognito-audience" + portal_cognito_trust_param = "${local.ssm_prefix}/portal-cognito-trust" # shared/slack.py resolves the token by NAME, while the IAM grant is scoped to # the ARN in var.slack_bot_secret_arn. Both must refer to the same secret. diff --git a/terraform/ssm.tf b/terraform/ssm.tf index 5d77646..337c249 100644 --- a/terraform/ssm.tf +++ b/terraform/ssm.tf @@ -25,6 +25,16 @@ resource "aws_ssm_parameter" "portal_cognito_audience" { description = "Trusted portal Cognito app client ID for ID-token verification" } +resource "aws_ssm_parameter" "portal_cognito_trust" { + name = local.portal_cognito_trust_param + type = "String" + value = jsonencode(concat( + [{ issuer = var.portal_cognito_issuer, audience = var.portal_cognito_audience }], + var.portal_cognito_extra_trust, + )) + description = "Trusted portal Cognito issuer/audience pairs for ID-token verification" +} + # --------------------------------------------------------------------------- # Deploy-time lookups # --------------------------------------------------------------------------- diff --git a/terraform/terraform.tfvars.example b/terraform/terraform.tfvars.example index 7c1c53d..0930a9b 100644 --- a/terraform/terraform.tfvars.example +++ b/terraform/terraform.tfvars.example @@ -17,7 +17,14 @@ slack_bot_secret_arn = "arn:aws:secretsmanager:us-east-1:011934824531:secret:mea # /meal-order-manager/slack-channel-id. slack_channel_id = "C00000000000" -# Portal Cognito pool and public app client accepted by the meals API. Use the -# dev pool during portal validation, then switch both values together at cutover. +# Portal Cognito pools and public app clients accepted by the meals API. +# The primary pair is required. extra_trust lists additional pools so +# portal-dev and portal-prod tokens can both call this prod meals stack. portal_cognito_issuer = "https://cognito-idp.us-east-1.amazonaws.com/us-east-1_EXAMPLE" portal_cognito_audience = "examplepublicappclientid" +portal_cognito_extra_trust = [ + { + issuer = "https://cognito-idp.us-east-1.amazonaws.com/us-east-1_EXAMPLEPROD" + audience = "exampleprodappclientid" + }, +] diff --git a/terraform/variables.tf b/terraform/variables.tf index 2687da3..fef5bc9 100644 --- a/terraform/variables.tf +++ b/terraform/variables.tf @@ -51,6 +51,25 @@ variable "portal_cognito_audience" { } } +variable "portal_cognito_extra_trust" { + description = "Additional portal Cognito issuer/audience pairs trusted by the meals API. Use this so portal-dev and portal-prod tokens both work against the single prod meals stack." + type = list(object({ + issuer = string + audience = string + })) + default = [] + + validation { + condition = alltrue([ + for pair in var.portal_cognito_extra_trust : ( + can(regex("^https://cognito-idp\\.[a-z0-9-]+\\.amazonaws\\.com/[A-Za-z0-9_-]+$", pair.issuer)) + && length(trimspace(pair.audience)) > 0 + ) + ]) + error_message = "Each extra trust entry must be a Cognito issuer URL without a trailing slash and a non-empty audience." + } +} + variable "checkcomponents_queue_url" { description = "paychex-checkcomponents SQS URL. Empty skips the weekly SendMessage." type = string diff --git a/tests/test_cognito.py b/tests/test_cognito.py index 340224e..a2ecec1 100644 --- a/tests/test_cognito.py +++ b/tests/test_cognito.py @@ -1,5 +1,6 @@ """Unit tests for portal Cognito ID-token verification.""" +import json import time from types import SimpleNamespace from unittest.mock import MagicMock, patch @@ -12,8 +13,11 @@ from shared import cognito ISSUER = "https://cognito-idp.us-east-1.amazonaws.com/us-east-1_TEST" AUDIENCE = "portal-client-id" +ISSUER_2 = "https://cognito-idp.us-east-1.amazonaws.com/us-east-1_PROD" +AUDIENCE_2 = "portal-prod-client" ISSUER_PARAM = "/meal-order-manager/portal-cognito-issuer" AUDIENCE_PARAM = "/meal-order-manager/portal-cognito-audience" +TRUST_PARAM = "/meal-order-manager/portal-cognito-trust" @pytest.fixture(scope="module") @@ -31,7 +35,7 @@ def _claims(**overrides): "exp": now + 300, "iat": now, "email": "employee@seahavenind.com", - "email_verified": True, + "email_verified": False, "name": "Test Employee", } claims.update(overrides) @@ -49,17 +53,28 @@ def _token(private_key, **overrides): def _parameter(name, decrypt=False): assert decrypt is False - return {ISSUER_PARAM: ISSUER, AUDIENCE_PARAM: AUDIENCE}[name] + return { + ISSUER_PARAM: ISSUER, + AUDIENCE_PARAM: AUDIENCE, + TRUST_PARAM: json.dumps( + [ + {"issuer": ISSUER, "audience": AUDIENCE}, + {"issuer": ISSUER_2, "audience": AUDIENCE_2}, + ] + ), + }[name] -def _verify(token, public_key): +def _verify(token, public_key, trust_param=""): jwks = MagicMock() jwks.get_signing_key_from_jwt.return_value = SimpleNamespace(key=public_key) with ( patch("shared.cognito.get_parameter", side_effect=_parameter), patch("shared.cognito._jwk_client", return_value=jwks), ): - return cognito.verify_cognito_id_token(token, ISSUER_PARAM, AUDIENCE_PARAM) + return cognito.verify_cognito_id_token( + token, ISSUER_PARAM, AUDIENCE_PARAM, trust_param + ) def test_valid_id_token_returns_trusted_identity(signing_key): @@ -73,6 +88,66 @@ def test_valid_id_token_returns_trusted_identity(signing_key): } +def test_unverified_federated_email_is_accepted(signing_key): + private_key, public_key = signing_key + + result = _verify(_token(private_key, email_verified=False), public_key) + + assert result["email"] == "employee@seahavenind.com" + + +def test_missing_name_falls_back_to_given_name(signing_key): + private_key, public_key = signing_key + claims = _claims() + del claims["name"] + claims["given_name"] = "Ada" + token = jwt.encode( + claims, private_key, algorithm="RS256", headers={"kid": "test-key"} + ) + + assert _verify(token, public_key) == { + "name": "Ada", + "email": "employee@seahavenind.com", + } + + +def test_missing_name_falls_back_to_email_local_part(signing_key): + private_key, public_key = signing_key + claims = _claims() + del claims["name"] + token = jwt.encode( + claims, private_key, algorithm="RS256", headers={"kid": "test-key"} + ) + + assert _verify(token, public_key) == { + "name": "employee", + "email": "employee@seahavenind.com", + } + + +def test_trust_list_accepts_second_issuer(signing_key): + private_key, public_key = signing_key + token = _token(private_key, iss=ISSUER_2, aud=AUDIENCE_2) + + result = _verify(token, public_key, trust_param=TRUST_PARAM) + + assert result == { + "name": "Test Employee", + "email": "employee@seahavenind.com", + } + + +def test_untrusted_issuer_is_rejected(signing_key): + private_key, public_key = signing_key + token = _token( + private_key, + iss="https://cognito-idp.us-east-1.amazonaws.com/us-east-1_OTHER", + aud=AUDIENCE, + ) + + assert _verify(token, public_key, trust_param=TRUST_PARAM) is None + + @pytest.mark.parametrize( "overrides", [ @@ -81,8 +156,6 @@ def test_valid_id_token_returns_trusted_identity(signing_key): {"token_use": "access"}, {"exp": int(time.time()) - 60}, {"email": "employee@example.com"}, - {"email_verified": False}, - {"name": ""}, ], ) def test_invalid_claims_are_rejected(signing_key, overrides): diff --git a/tests/test_terraform_cognito_auth.py b/tests/test_terraform_cognito_auth.py index 6ef8163..0aa3911 100644 --- a/tests/test_terraform_cognito_auth.py +++ b/tests/test_terraform_cognito_auth.py @@ -23,10 +23,16 @@ def test_portal_cognito_parameters_are_managed(): 'portal_cognito_audience_param = "${local.ssm_prefix}/portal-cognito-audience"' in locals_tf ) + assert ( + 'portal_cognito_trust_param = "${local.ssm_prefix}/portal-cognito-trust"' + in locals_tf + ) assert 'resource "aws_ssm_parameter" "portal_cognito_issuer"' in ssm_tf assert 'resource "aws_ssm_parameter" "portal_cognito_audience"' in ssm_tf + assert 'resource "aws_ssm_parameter" "portal_cognito_trust"' in ssm_tf assert 'variable "portal_cognito_issuer"' in variables_tf assert 'variable "portal_cognito_audience"' in variables_tf + assert 'variable "portal_cognito_extra_trust"' in variables_tf def test_both_api_lambdas_receive_parameter_names(): @@ -34,8 +40,10 @@ def test_both_api_lambdas_receive_parameter_names(): assert lambda_tf.count("PORTAL_COGNITO_ISSUER_PARAM") == 2 assert lambda_tf.count("PORTAL_COGNITO_AUDIENCE_PARAM") == 2 + assert lambda_tf.count("PORTAL_COGNITO_TRUST_PARAM") == 2 assert lambda_tf.count("aws_ssm_parameter.portal_cognito_issuer.name") == 2 assert lambda_tf.count("aws_ssm_parameter.portal_cognito_audience.name") == 2 + assert lambda_tf.count("aws_ssm_parameter.portal_cognito_trust.name") == 2 def test_submit_route_remains_public_for_google_compatibility():