diff --git a/docs/app-yaml-reference.md b/docs/app-yaml-reference.md index f1a16e2..b6f6dc9 100644 --- a/docs/app-yaml-reference.md +++ b/docs/app-yaml-reference.md @@ -216,6 +216,10 @@ auth: COGNITO_ISSUER_URL: NUXT_PUBLIC_OIDC_AUTHORITY ``` +#### `scopes` + +Any of `openid`, `email`, `profile`, `phone`, `aws.cognito.signin.user.admin`. The `internal` pool refuses `aws.cognito.signin.user.admin`: it lets a signed-in user change their own attributes, including the email the platform uses to look up group membership. A request with it is rejected and the client is not created or changed. + #### `client` | Value | For | You get | @@ -232,7 +236,7 @@ Changing `client` between `confidential` and `public` replaces the Cognito clien Both forms are supported and produce the same wiring. -- `internal` — Javabin heroes. Sign-in offers every identity provider on the pool (Google Workspace SAML) plus Cognito users. +- `internal` — Javabin heroes. The hosted UI sends users straight to Google Workspace (`GoogleSAML`), so apps don't need an `identity_provider` parameter. There is no password form or sign-up. Only `@java.no` accounts get a token; anyone else is refused at sign-in. - `external` — public users. - `none` or omitted — no Cognito integration. @@ -271,7 +275,7 @@ On the very first deploy with `auth:`, the first task can start a few seconds be Tokens from the internal pool carry the user's registry groups in `cognito:groups` (ID and access token): every group in `javaBin/registry` `groups/groups.yaml` with `cognito: true` the user is a member of, plus `team-{team}` for teams they belong to. The `javabin-auth-claims` trigger adds them at every sign-in and refresh, from membership team-provisioner records on each registry merge. A registry change reaches tokens at the user's next sign-in or token refresh. -`auth.groups` restricts the client: a user in none of the listed groups is refused at sign-in, so the app never receives a token for them. Apps should still check `cognito:groups` for finer-grained authorization (e.g. admin features). +`auth.groups` restricts the client: a user in none of the listed groups is refused at sign-in, so the app never receives a token for them. The refusal (like the internal pool's `@java.no` check) comes back to your callback URL as `error=…&error_description=…PreTokenGeneration failed…`; show `error_description` to the user rather than a blank page. Apps should still check `cognito:groups` for finer-grained authorization (e.g. admin features). No Cognito groups are created from `app.yaml`; add groups in the registry. diff --git a/docs/lambda-functions.md b/docs/lambda-functions.md index 3ee17e9..9049982 100644 --- a/docs/lambda-functions.md +++ b/docs/lambda-functions.md @@ -125,7 +125,11 @@ Idempotent: skips resources that already have a `created-by` tag (preserves orig **Trigger:** EventBridge "Parameter Store Change" for `/javabin/apps/*/*/auth` (Create/Update/Delete), plus a daily `{"action": "reconcile"}` at 03:30 UTC **Purpose:** Creates, updates and deletes the Cognito app clients requested by `app.yaml` `auth:`. The request parameter's path identifies the team (only that team can write there), so no caller-supplied identity is trusted. -**Flow:** Re-read the request → validate (pool, client_type, https/localhost URLs, scopes, allowed_groups) → upsert client `{team}-{app}` in the requested pool with every pool IdP + `COGNITO` (a client_type change replaces the client, since the secret setting is immutable) → remove it from the other pool → write `/javabin/apps/{team}/{app}/cognito-client-id` (String) and, for confidential clients, `cognito-client-secret` (SecureString), tagged `team={team}` → record `allowed_groups` in `javabin-auth-memberships`. A missing request deletes the client and both parameters. Reconcile also removes clients whose credential parameter exists but whose request is gone; clients created by other means are never touched. +**Flow:** Re-read the request → validate (pool, client_type, https/localhost URLs, scopes, allowed_groups) → upsert client `{team}-{app}` in the requested pool with that pool's fixed sign-in IdP list (a client_type change replaces the client, since the secret setting is immutable) → remove it from the other pool → write `/javabin/apps/{team}/{app}/cognito-client-id` (String) and, for confidential clients, `cognito-client-secret` (SecureString), tagged `team={team}` → record `allowed_groups` in `javabin-auth-memberships`. A missing request deletes the client and both parameters. Reconcile also removes clients whose credential parameter exists but whose request is gone; clients created by other means are never touched. + +**Identity providers:** every client in a pool gets exactly the pool's list, from `INTERNAL_IDENTITY_PROVIDERS` / `EXTERNAL_IDENTITY_PROVIDERS` (comma-separated; set from the identity module's `internal_identity_providers` = `["GoogleSAML"]` and `external_identity_providers` = `["COGNITO"]`). With a single IdP the hosted UI redirects straight to it, so internal apps show no password form or sign-up link. Reconcile overwrites console edits to the list. If a configured IdP does not exist in the pool, the request is skipped with an `ERROR` log and status `error` (reconcile lists it under `error`): nothing is created, moved or changed, and the existing client keeps working. The CloudWatch alarm `javabin-app-auth-sync-skipped` (metric filter on `client left unchanged`) posts to `javabin-alerts` when this happens. Internal-pool requests asking for `aws.cognito.signin.user.admin` are rejected as invalid (that scope would let a user rewrite the email auth-claims trusts). + +**Environment vars:** `PROJECT`, `MEMBERSHIPS_TABLE`, `INTERNAL_IDENTITY_PROVIDERS`, `EXTERNAL_IDENTITY_PROVIDERS` **Concurrency:** reserved concurrency 1, so two changes to one app cannot race into duplicate clients. @@ -134,7 +138,11 @@ Idempotent: skips resources that already have a `created-by` tag (preserves orig ## auth-claims **Trigger:** Cognito pre-token-generation (V2) on the internal pool (`javabin-internal`), every sign-in and token refresh -**Purpose:** Puts the user's registry groups into `cognito:groups` (ID and access token), and refuses sign-in when the app client has `allowed_groups` and the user is in none of them. +**Purpose:** Refuses tokens for accounts outside the Workspace domain, puts the user's registry groups into `cognito:groups` (ID and access token), and refuses sign-in when the app client has `allowed_groups` and the user is in none of them. + +**Domain check:** with `REQUIRED_EMAIL_DOMAIN` set (internal pool: `var.google_workspace_domain`, `java.no`), a token is issued only when the email ends with `@java.no` (case-insensitive) **and** the user signed in through one of `FEDERATED_IDENTITY_PROVIDERS` (the internal pool's IdP list, `GoogleSAML`) or has `email_verified=true`. Federated users are recognised by the `identities` attribute, or by their `_` username. Anyone else gets "Sign in with your @java.no account." on the app's callback. Every IdP in that list is trusted to vouch for the email (no `email_verified` check), so it must release only Workspace accounts; see the comment on `internal_identity_providers` before adding one (for example a Google OIDC IdP). A function without `REQUIRED_EMAIL_DOMAIN` skips the check. The internal pool also has self sign-up disabled (`allow_admin_create_user_only`). + +**Environment vars:** `MEMBERSHIPS_TABLE`, `REQUIRED_EMAIL_DOMAIN`, `FEDERATED_IDENTITY_PROVIDERS` **Data:** `javabin-auth-memberships` (DynamoDB). `group#` items (members' emails) are written by team-provisioner on registry merges; `client#` items (`allowed_groups`) by app-auth-sync. Memberships are cached for 60 s per container. diff --git a/terraform/lambda-src/app_auth_sync/handler.py b/terraform/lambda-src/app_auth_sync/handler.py index 7fa166a..5a5df3e 100644 --- a/terraform/lambda-src/app_auth_sync/handler.py +++ b/terraform/lambda-src/app_auth_sync/handler.py @@ -14,6 +14,13 @@ where the app's ECS task picks them up via its team's execution role. Public clients (browser apps using PKCE) get no secret and no secret parameter. +Each pool has a fixed sign-in IdP list (INTERNAL_IDENTITY_PROVIDERS / +EXTERNAL_IDENTITY_PROVIDERS, comma-separated, set by Terraform). Every client +in the pool gets exactly that list, so with a single IdP the hosted UI sends +the user straight to it (no password form, no sign-up link). If a configured +IdP does not exist in the pool, the request is skipped and logged as an error; +the existing client is left as it is. + The request's allowed groups are written to the auth memberships table as {pk: "client#", allowed_groups: [...]}; the auth-claims trigger refuses tokens for users in none of them. @@ -40,12 +47,27 @@ "internal": os.environ.get("INTERNAL_POOL_NAME", f"{PROJECT}-internal"), "external": os.environ.get("EXTERNAL_POOL_NAME", f"{PROJECT}-external"), } + +def _idp_list(value): + return [p.strip() for p in value.split(",") if p.strip()] + + +# Sign-in IdPs per pool. COGNITO means local username/password users. +POOL_IDPS = { + "internal": _idp_list(os.environ.get("INTERNAL_IDENTITY_PROVIDERS", "GoogleSAML")), + "external": _idp_list(os.environ.get("EXTERNAL_IDENTITY_PROVIDERS", "COGNITO")), +} MEMBERSHIPS_TABLE = os.environ.get("MEMBERSHIPS_TABLE", "") DEFAULT_SCOPES = ["openid", "email", "profile"] CLIENT_TYPES = ("confidential", "public") # Cognito allows plain http only for localhost, for local development LOCAL_URL = re.compile(r"^http://(localhost|127\.0\.0\.1)(:\d+)?(/.*)?$") ALLOWED_SCOPES = {"openid", "email", "profile", "phone", "aws.cognito.signin.user.admin"} +# aws.cognito.signin.user.admin lets the signed-in user call UpdateUserAttributes +# with their access token. In the internal pool, auth-claims trusts a federated +# user's email and looks up registry groups by it, so a user who could rewrite +# their own email could take another user's groups. +POOL_FORBIDDEN_SCOPES = {"internal": {"aws.cognito.signin.user.admin"}} APPS_PREFIX = f"/{PROJECT}/apps/" REQUEST_RE = re.compile(rf"^/{re.escape(PROJECT)}/apps/(?P[a-z]{{1,12}})/(?P[a-z][a-z0-9-]{{0,19}})/auth$") @@ -59,6 +81,10 @@ class InvalidRequest(ValueError): pass +class PoolMisconfigured(RuntimeError): + """The platform side is wrong (not the app's request): skip, keep the client.""" + + # --------------------------------------------------------------------------- # Request parsing # --------------------------------------------------------------------------- @@ -95,6 +121,9 @@ def urls(key, required): scopes = req.get("scopes") or DEFAULT_SCOPES if not isinstance(scopes, list) or not set(scopes) <= ALLOWED_SCOPES: raise InvalidRequest(f"scopes must be a subset of {sorted(ALLOWED_SCOPES)}") + forbidden = set(scopes) & POOL_FORBIDDEN_SCOPES.get(pool, set()) + if forbidden: + raise InvalidRequest(f"scopes {sorted(forbidden)} are not allowed in the {pool} pool") client_type = req.get("client_type") or "confidential" if client_type not in CLIENT_TYPES: @@ -141,15 +170,27 @@ def find_client(user_pool_id, client_name): return None -def identity_providers(user_pool_id): - """Every IdP configured on the pool (e.g. GoogleSAML) plus local users.""" - names = [] - for page in cognito.get_paginator("list_identity_providers").paginate(UserPoolId=user_pool_id, MaxResults=60): - names += [p["ProviderName"] for p in page["Providers"]] - return names + ["COGNITO"] +_idp_cache = {} + + +def identity_providers(pool, user_pool_id): + """The pool's configured sign-in IdPs, after checking each one exists.""" + wanted = POOL_IDPS[pool] + if not wanted: + raise PoolMisconfigured(f"no identity providers configured for the {pool} pool") + if user_pool_id not in _idp_cache: + names = set() + for page in cognito.get_paginator("list_identity_providers").paginate(UserPoolId=user_pool_id, MaxResults=60): + names |= {p["ProviderName"] for p in page["Providers"]} + _idp_cache[user_pool_id] = names + existing = _idp_cache[user_pool_id] + missing = [p for p in wanted if p != "COGNITO" and p not in existing] + if missing: + raise PoolMisconfigured(f"{pool} pool has no identity provider {missing}; it has {sorted(existing)}") + return list(wanted) -def client_settings(user_pool_id, client_name, req): +def client_settings(pool, user_pool_id, client_name, req): return { "UserPoolId": user_pool_id, "ClientName": client_name, @@ -158,7 +199,7 @@ def client_settings(user_pool_id, client_name, req): "AllowedOAuthScopes": req["scopes"], "CallbackURLs": req["callback_urls"], "LogoutURLs": req["logout_urls"], - "SupportedIdentityProviders": identity_providers(user_pool_id), + "SupportedIdentityProviders": identity_providers(pool, user_pool_id), "ExplicitAuthFlows": ["ALLOW_REFRESH_TOKEN_AUTH", "ALLOW_USER_SRP_AUTH"], "PreventUserExistenceErrors": "ENABLED", } @@ -195,6 +236,10 @@ def _put_param(name, value, secure, team, app): def sync(team, app, req): """Make Cognito match one request. Returns the client ID.""" client_name = f"{team}-{app}" + # Resolve settings first: a misconfigured pool must abort before anything + # (including the pool-switch cleanup below) is changed. + upid = pool_id(req["pool"]) + settings = client_settings(req["pool"], upid, client_name, req) # A pool switch (internal <-> external) leaves no client behind. for other in POOL_NAMES: @@ -206,8 +251,6 @@ def sync(team, app, req): _delete_client_groups(stale) logger.info("Removed %s from %s pool", client_name, other) - upid = pool_id(req["pool"]) - settings = client_settings(upid, client_name, req) want_secret = req["client_type"] == "confidential" client_id = find_client(upid, client_name) if client_id: @@ -298,7 +341,12 @@ def handle_change(name, operation): except InvalidRequest as e: logger.error("Invalid auth request %s: %s", name, e) return {"status": "invalid", "error": str(e)} - return {"status": "synced", "client": f"{team}-{app}", "client_id": sync(team, app, req)} + try: + client_id = sync(team, app, req) + except PoolMisconfigured as e: + logger.error("Skipped %s-%s, client left unchanged: %s", team, app, e) + return {"status": "error", "error": str(e)} + return {"status": "synced", "client": f"{team}-{app}", "client_id": client_id} def reconcile(): @@ -311,7 +359,7 @@ def reconcile(): if owner: wanted[owner] = p["Value"] - results = {"synced": [], "invalid": [], "deleted": []} + results = {"synced": [], "invalid": [], "error": [], "deleted": []} for (team, app), raw in sorted(wanted.items()): try: sync(team, app, parse_request(raw)) @@ -319,6 +367,9 @@ def reconcile(): except InvalidRequest as e: logger.error("Invalid auth request for %s-%s: %s", team, app, e) results["invalid"].append(f"{team}-{app}") + except PoolMisconfigured as e: + logger.error("Skipped %s-%s, client left unchanged: %s", team, app, e) + results["error"].append(f"{team}-{app}") # Orphans: clients whose credential parameter exists (so this Lambda made # them) but whose request is gone. Clients created by other means are @@ -335,6 +386,8 @@ def reconcile(): def handler(event, context): + # IdPs can be added or removed between invocations of a warm container + _idp_cache.clear() if event.get("action") == "reconcile": return reconcile() diff --git a/terraform/lambda-src/auth_claims/handler.py b/terraform/lambda-src/auth_claims/handler.py index b40eb76..4508405 100644 --- a/terraform/lambda-src/auth_claims/handler.py +++ b/terraform/lambda-src/auth_claims/handler.py @@ -15,8 +15,17 @@ refuse the sign-in, so the app never receives a token. Clients without an entry, or with an empty list, accept any pool user. + +When REQUIRED_EMAIL_DOMAIN is set (internal pool only, e.g. java.no), the +trigger first refuses every user whose email is not @ (case-insensitive) +or whose address is not vouched for: the user must come from one of +FEDERATED_IDENTITY_PROVIDERS (the pool's sign-in IdPs, which only release +Workspace accounts, e.g. GoogleSAML) or have email_verified=true. This is the +server-side guard against self-registered or foreign-IdP users in the internal +pool; the IdP's hosted-domain hint is only a UI hint. """ +import json import logging import os import time @@ -28,6 +37,9 @@ TABLE = os.environ.get("MEMBERSHIPS_TABLE", "javabin-auth-memberships") CACHE_SECONDS = int(os.environ.get("CACHE_SECONDS", "60")) +REQUIRED_DOMAIN = os.environ.get("REQUIRED_EMAIL_DOMAIN", "").strip().lower().lstrip("@") +FEDERATED_IDPS = {p.strip() for p in os.environ.get("FEDERATED_IDENTITY_PROVIDERS", "").split(",") + if p.strip() and p.strip() != "COGNITO"} dynamodb = boto3.client("dynamodb") @@ -64,14 +76,56 @@ def group_members(): return _cache["groups"] +def _masked(email): + """e***@java.no — enough to correlate in logs without storing the address.""" + if "@" not in email: + return "" + local, domain = email.split("@", 1) + return f"{local[:1]}***@{domain}" + + def allowed_groups(client_id): item = dynamodb.get_item(TableName=TABLE, Key={"pk": {"S": f"client#{client_id}"}}).get("Item") return set(_strings(item.get("allowed_groups"))) if item else set() +def _providers(event, attrs): + """Names of the federated IdPs this user signed in through (empty for native users).""" + names = set() + try: + identities = json.loads(attrs.get("identities") or "[]") + except (TypeError, ValueError): + identities = [] + if isinstance(identities, list): + names |= {i.get("providerName") for i in identities if isinstance(i, dict) and i.get("providerName")} + # Federated usernames are "_"; covers events without + # identities. Not spoofable: the pool uses email as the sign-in alias, so + # Cognito assigns native users a UUID username. + user_name = event.get("userName") or "" + names |= {p for p in FEDERATED_IDPS if user_name.startswith(p + "_")} + return names + + +def domain_allowed(event, attrs, email): + """True when the pool has no domain rule, or the user satisfies it.""" + if not REQUIRED_DOMAIN: + return True + if not email.endswith("@" + REQUIRED_DOMAIN): + return False + if _providers(event, attrs) & FEDERATED_IDPS: + return True + return str(attrs.get("email_verified", "")).lower() == "true" + + def handler(event, context): request = event.get("request", {}) - email = (request.get("userAttributes", {}).get("email") or "").lower() + attrs = request.get("userAttributes") or {} + email = (attrs.get("email") or "").lower() + + if not domain_allowed(event, attrs, email): + logger.info("Denied %s: not a verified @%s account", _masked(email), REQUIRED_DOMAIN) + raise AccessDenied(f"Sign in with your @{REQUIRED_DOMAIN} account.") + client_id = event.get("callerContext", {}).get("clientId", "") native = (request.get("groupConfiguration") or {}).get("groupsToOverride") or [] @@ -80,7 +134,7 @@ def handler(event, context): allowed = allowed_groups(client_id) if client_id else set() if allowed and not allowed & set(groups): - logger.info("Denied %s for client %s: needs one of %s", email or "", client_id, sorted(allowed)) + logger.info("Denied %s for client %s: needs one of %s", _masked(email), client_id, sorted(allowed)) raise AccessDenied("You are not in a group that has access to this app.") event["response"] = { diff --git a/terraform/lambda-src/tests/test_app_auth_sync.py b/terraform/lambda-src/tests/test_app_auth_sync.py index 40ab733..01f9d98 100644 --- a/terraform/lambda-src/tests/test_app_auth_sync.py +++ b/terraform/lambda-src/tests/test_app_auth_sync.py @@ -99,8 +99,14 @@ def describe_user_pool_client(self, UserPoolId, ClientId): return {"UserPoolClient": self.clients[ClientId]} -def load_handler(ssm, cognito, dynamo=None): +IDP_ENV = ("INTERNAL_IDENTITY_PROVIDERS", "EXTERNAL_IDENTITY_PROVIDERS") + + +def load_handler(ssm, cognito, dynamo=None, **idps): os.environ["MEMBERSHIPS_TABLE"] = "javabin-auth-memberships" + for key in IDP_ENV: + os.environ.pop(key, None) + os.environ.update(idps) boto3 = types.ModuleType("boto3") boto3.client = lambda svc: {"ssm": ssm, "cognito-idp": cognito, "dynamodb": dynamo or FakeDynamo()}[svc] sys.modules["boto3"] = boto3 @@ -140,7 +146,8 @@ def test_create_makes_client_and_writes_team_tagged_credentials(self): cid, client = self.only_client() self.assertEqual(client["ClientName"], "cupcake-frosting") self.assertEqual(client["UserPoolId"], "pool-int") - self.assertEqual(client["SupportedIdentityProviders"], ["GoogleSAML", "COGNITO"]) + # Only the pool's IdP: no COGNITO, so no password form or sign-up link + self.assertEqual(client["SupportedIdentityProviders"], ["GoogleSAML"]) self.assertEqual(self.ssm.params["/javabin/apps/cupcake/frosting/cognito-client-id"], cid) self.assertEqual(self.ssm.params["/javabin/apps/cupcake/frosting/cognito-client-secret"], client["ClientSecret"]) self.assertEqual(self.ssm.tags["/javabin/apps/cupcake/frosting/cognito-client-id"]["team"], "cupcake") @@ -200,6 +207,21 @@ def test_invalid_request_creates_nothing(self): self.assertEqual(self.h.handler(event(REQ, "Update"), None)["status"], "invalid") self.assertEqual(self.cognito.clients, {}) + def test_internal_pool_refuses_the_user_admin_scope(self): + # The scope would let a user rewrite their own email, which auth-claims + # trusts for federated users when it looks up registry groups. + self.put_request({**REQUEST, "pool": "internal", + "scopes": ["openid", "email", "aws.cognito.signin.user.admin"]}) + result = self.h.handler(event(REQ, "Create"), None) + self.assertEqual(result["status"], "invalid") + self.assertIn("aws.cognito.signin.user.admin", result["error"]) + self.assertEqual(self.cognito.clients, {}) + + def test_external_pool_still_allows_the_user_admin_scope(self): + self.put_request({**REQUEST, "pool": "external", + "scopes": ["openid", "email", "aws.cognito.signin.user.admin"]}) + self.assertEqual(self.h.handler(event(REQ, "Create"), None)["status"], "synced") + def test_reconcile_syncs_requests_and_removes_orphans(self): self.put_request() other = "/javabin/apps/core/moresleep/auth" @@ -215,6 +237,77 @@ def test_reconcile_syncs_requests_and_removes_orphans(self): self.assertEqual(client["ClientName"], "cupcake-frosting") +class IdentityProviderTest(unittest.TestCase): + ID_PARAM = "/javabin/apps/cupcake/frosting/cognito-client-id" + + def setUp(self): + self.ssm, self.cognito, self.dynamo = FakeSSM(), FakeCognito(), FakeDynamo() + self.h = load_handler(self.ssm, self.cognito, self.dynamo) + self.ssm.params[REQ] = json.dumps(REQUEST) + self.h.handler(event(REQ, "Create"), None) + self.cid = next(iter(self.cognito.clients)) + + def tearDown(self): + for key in IDP_ENV: + os.environ.pop(key, None) + + def reload(self, **idps): + self.h = load_handler(self.ssm, self.cognito, self.dynamo, **idps) + + def client(self): + return self.cognito.clients[self.cid] + + def test_reconcile_restores_the_pool_idp_list(self): + self.client()["SupportedIdentityProviders"] = ["GoogleSAML", "COGNITO"] + self.h.handler({"action": "reconcile"}, None) + self.assertEqual(self.client()["SupportedIdentityProviders"], ["GoogleSAML"]) + + def test_idp_list_comes_from_the_environment(self): + self.reload(INTERNAL_IDENTITY_PROVIDERS="GoogleSAML, COGNITO") + self.assertEqual(self.h.handler(event(REQ, "Update"), None)["status"], "synced") + self.assertEqual(self.client()["SupportedIdentityProviders"], ["GoogleSAML", "COGNITO"]) + + def test_missing_idp_skips_the_request_and_keeps_the_client(self): + self.reload(INTERNAL_IDENTITY_PROVIDERS="Google") + self.ssm.params[REQ] = json.dumps({**REQUEST, "callback_urls": ["https://new.javazone.no/cb"]}) + with self.assertLogs(level="ERROR"): + result = self.h.handler(event(REQ, "Update"), None) + self.assertEqual(result["status"], "error") + self.assertIn("Google", result["error"]) + self.assertEqual(list(self.cognito.clients), [self.cid]) + self.assertEqual(self.client()["SupportedIdentityProviders"], ["GoogleSAML"]) + self.assertEqual(self.client()["CallbackURLs"], REQUEST["callback_urls"]) + self.assertEqual(self.ssm.params[self.ID_PARAM], self.cid) + + def test_missing_idp_creates_no_client(self): + self.reload(INTERNAL_IDENTITY_PROVIDERS="Google") + other = "/javabin/apps/core/moresleep/auth" + self.ssm.params[other] = json.dumps(REQUEST) + self.assertEqual(self.h.handler(event(other, "Create"), None)["status"], "error") + self.assertEqual(list(self.cognito.clients), [self.cid]) + self.assertNotIn("/javabin/apps/core/moresleep/cognito-client-id", self.ssm.params) + + def test_empty_idp_list_is_an_error_not_a_client_nobody_can_use(self): + self.reload(INTERNAL_IDENTITY_PROVIDERS=" , ") + self.assertEqual(self.h.handler(event(REQ, "Update"), None)["status"], "error") + self.assertEqual(self.client()["SupportedIdentityProviders"], ["GoogleSAML"]) + + def test_missing_idp_in_target_pool_does_not_move_the_client(self): + self.reload(EXTERNAL_IDENTITY_PROVIDERS="Google") + self.ssm.params[REQ] = json.dumps({**REQUEST, "pool": "external"}) + self.assertEqual(self.h.handler(event(REQ, "Update"), None)["status"], "error") + self.assertEqual(self.client()["UserPoolId"], "pool-int") + + def test_reconcile_reports_and_keeps_skipped_clients(self): + self.reload(INTERNAL_IDENTITY_PROVIDERS="Google") + with self.assertLogs(level="ERROR"): + result = self.h.handler({"action": "reconcile"}, None) + self.assertEqual(result["error"], ["cupcake-frosting"]) + self.assertEqual(result["deleted"], []) + self.assertEqual(list(self.cognito.clients), [self.cid]) + self.assertEqual(self.ssm.params[self.ID_PARAM], self.cid) + + class AuthFeaturesTest(AppAuthSyncTest): SECRET = "/javabin/apps/cupcake/frosting/cognito-client-secret" diff --git a/terraform/lambda-src/tests/test_auth_claims.py b/terraform/lambda-src/tests/test_auth_claims.py index c607e3f..366518e 100644 --- a/terraform/lambda-src/tests/test_auth_claims.py +++ b/terraform/lambda-src/tests/test_auth_claims.py @@ -1,6 +1,7 @@ """Tests for auth_claims — run: python3 -m unittest discover terraform/lambda-src/tests""" import importlib.util +import json import os import sys import types @@ -29,7 +30,13 @@ def client(cid, *groups): return {"pk": {"S": f"client#{cid}"}, "allowed_groups": {"L": [{"S": g} for g in groups]}} -def load(dynamo): +DOMAIN_ENV = ("REQUIRED_EMAIL_DOMAIN", "FEDERATED_IDENTITY_PROVIDERS") + + +def load(dynamo, **env): + for key in DOMAIN_ENV: + os.environ.pop(key, None) + os.environ.update(env) boto3 = types.ModuleType("boto3") boto3.client = lambda svc: dynamo sys.modules["boto3"] = boto3 @@ -53,6 +60,72 @@ def groups_of(result): return result["response"]["claimsAndScopeOverrideDetails"]["groupOverrideDetails"]["groupsToOverride"] +def user(email, provider=None, verified=None, client_id="other-app"): + """A pre-token event shaped like Cognito's: federated users carry identities.""" + attrs = {"email": email} + e = event(email, client_id=client_id) + if provider: + attrs["identities"] = json.dumps([{"userId": email, "providerName": provider, "providerType": "SAML", + "issuer": None, "primary": "true", "dateCreated": "1"}]) + e["userName"] = f"{provider}_{email}" + else: + e["userName"] = "6f1c2e4a-0000-4000-8000-000000000000" + if verified is not None: + attrs["email_verified"] = verified + e["request"]["userAttributes"] = attrs + return e + + +class EmailDomainTest(unittest.TestCase): + def setUp(self): + self.dynamo = FakeDynamo([group("helter", "ada.lovelace@java.no"), client("frosting", "helter")]) + self.h = load(self.dynamo, REQUIRED_EMAIL_DOMAIN="java.no", FEDERATED_IDENTITY_PROVIDERS="GoogleSAML") + + def tearDown(self): + for key in DOMAIN_ENV: + os.environ.pop(key, None) + + def assertAllowed(self, e): + self.assertIn("claimsAndScopeOverrideDetails", self.h.handler(e, None)["response"]) + + def assertDenied(self, e): + with self.assertRaises(self.h.AccessDenied): + self.h.handler(e, None) + + def test_existing_saml_users_pass_without_email_verified(self): + # Live users: GoogleSAML, @java.no, email_verified=false + self.assertAllowed(user("Ada.Lovelace@Java.NO", provider="GoogleSAML", verified="false", client_id="frosting")) + + def test_saml_user_recognised_by_username_when_identities_is_missing(self): + e = user("ada.lovelace@java.no", provider="GoogleSAML", client_id="frosting") + del e["request"]["userAttributes"]["identities"] + self.assertAllowed(e) + + def test_self_registered_gmail_user_is_denied(self): + self.assertDenied(user("someone@gmail.com", verified="true")) + + def test_lookalike_domains_are_denied(self): + for email in ("x@java.no.evil.com", "x@notjava.no", "x@sub.java.no", "x@java.no@evil.com", "java.no", ""): + self.assertDenied(user(email, provider="GoogleSAML")) + + def test_native_java_no_user_needs_a_verified_email(self): + self.assertDenied(user("grace.hopper@java.no", verified="false")) + self.assertDenied(user("grace.hopper@java.no")) + self.assertAllowed(user("grace.hopper@java.no", verified="true")) + + def test_other_idp_needs_a_verified_email(self): + self.assertDenied(user("grace.hopper@java.no", provider="SomeOtherIdP", verified="false")) + self.assertAllowed(user("grace.hopper@java.no", provider="Google", verified="true")) + + def test_group_rule_still_applies_after_the_domain_check(self): + self.assertDenied(user("grace.hopper@java.no", provider="GoogleSAML", client_id="frosting")) + + def test_pool_without_a_domain_rule_accepts_any_email(self): + h = load(self.dynamo) + result = h.handler(user("someone@gmail.com"), None) + self.assertEqual(groups_of(result), []) + + class AuthClaimsTest(unittest.TestCase): def setUp(self): self.dynamo = FakeDynamo([ @@ -71,8 +144,10 @@ def test_native_cognito_groups_are_kept(self): self.assertEqual(groups_of(result), ["admins", "helter"]) def test_user_outside_allowed_groups_is_refused(self): - with self.assertRaises(self.h.AccessDenied): + with self.assertRaises(self.h.AccessDenied), self.assertLogs(self.h.logger, "INFO") as logs: self.h.handler(event("grace.hopper@java.no"), None) + # Denials log a masked address, never the full email + self.assertNotIn("grace.hopper@java.no", "\n".join(logs.output)) def test_client_without_restriction_accepts_anyone(self): result = self.h.handler(event("grace.hopper@java.no", client_id="other-app"), None) diff --git a/terraform/platform/identity/main.tf b/terraform/platform/identity/main.tf index e176986..d08739b 100644 --- a/terraform/platform/identity/main.tf +++ b/terraform/platform/identity/main.tf @@ -24,6 +24,12 @@ resource "aws_cognito_user_pool" "internal" { auto_verified_attributes = ["email"] username_attributes = ["email"] + # No self sign-up: users arrive through the federated IdP (or are created + # by an admin). auth-claims also refuses non-@java.no accounts. + admin_create_user_config { + allow_admin_create_user_only = true + } + password_policy { minimum_length = 12 require_lowercase = true @@ -281,6 +287,10 @@ resource "aws_lambda_function" "auth_claims" { environment { variables = { MEMBERSHIPS_TABLE = aws_dynamodb_table.auth_memberships.name + # Internal pool only: refuse tokens for anything but verified + # @ accounts. Users from these IdPs count as verified. + REQUIRED_EMAIL_DOMAIN = var.google_workspace_domain + FEDERATED_IDENTITY_PROVIDERS = join(",", var.internal_identity_providers) } } } diff --git a/terraform/platform/identity/outputs.tf b/terraform/platform/identity/outputs.tf index 149cdfe..a2eb944 100644 --- a/terraform/platform/identity/outputs.tf +++ b/terraform/platform/identity/outputs.tf @@ -53,3 +53,13 @@ output "auth_memberships_table_arn" { description = "ARN of the auth memberships table" value = aws_dynamodb_table.auth_memberships.arn } + +output "internal_identity_providers" { + description = "Sign-in IdPs app-auth-sync puts on internal-pool clients" + value = var.internal_identity_providers +} + +output "external_identity_providers" { + description = "Sign-in IdPs app-auth-sync puts on external-pool clients" + value = var.external_identity_providers +} diff --git a/terraform/platform/identity/variables.tf b/terraform/platform/identity/variables.tf index e6faa86..d6d0e73 100644 --- a/terraform/platform/identity/variables.tf +++ b/terraform/platform/identity/variables.tf @@ -43,3 +43,37 @@ variable "org_boundary_arn" { description = "Permissions boundary for roles created here (required by the org boundary)" type = string } + +# Sign-in IdPs per pool. app-auth-sync puts exactly this list on every app +# client; with one IdP the hosted UI redirects straight to it. "COGNITO" means +# local username/password users. Every other name must exist in the pool, or +# app-auth-sync skips the request (and logs an error) instead of changing it. +# +# Internal pool: auth-claims also trusts every IdP in this list to vouch for the +# user's email (no email_verified check) and looks up registry groups by it. Only +# list IdPs that release java.no Workspace accounts and nothing else: GoogleSAML +# does. A Google OIDC IdP qualifies only if its GCP OAuth client is Internal +# (Workspace-only); otherwise hd= is just a hint and a consumer Google account +# with an @java.no address would pass. In that case map email_verified/hd and +# check them in auth-claims before adding the IdP here. +variable "internal_identity_providers" { + description = "Identity providers on internal-pool app clients (GoogleSAML is created outside Terraform)" + type = list(string) + default = ["GoogleSAML"] + + validation { + condition = length(var.internal_identity_providers) > 0 && !contains(var.internal_identity_providers, "COGNITO") + error_message = "The internal pool needs at least one federated IdP and must not offer COGNITO (local sign-up)." + } +} + +variable "external_identity_providers" { + description = "Identity providers on external-pool app clients" + type = list(string) + default = ["COGNITO"] + + validation { + condition = length(var.external_identity_providers) > 0 + error_message = "The external pool needs at least one identity provider." + } +} diff --git a/terraform/platform/lambdas/main.tf b/terraform/platform/lambdas/main.tf index 171fbeb..00c101c 100644 --- a/terraform/platform/lambdas/main.tf +++ b/terraform/platform/lambdas/main.tf @@ -1682,8 +1682,10 @@ resource "aws_lambda_function" "app_auth_sync" { environment { variables = { - PROJECT = var.project - MEMBERSHIPS_TABLE = var.auth_memberships_table_name + PROJECT = var.project + MEMBERSHIPS_TABLE = var.auth_memberships_table_name + INTERNAL_IDENTITY_PROVIDERS = join(",", var.internal_identity_providers) + EXTERNAL_IDENTITY_PROVIDERS = join(",", var.external_identity_providers) } } } @@ -1739,6 +1741,38 @@ resource "aws_lambda_permission" "app_auth_sync_reconcile" { source_arn = aws_cloudwatch_event_rule.app_auth_sync_reconcile.arn } +# A skipped request (configured IdP missing from the pool) leaves the client +# unchanged and returns normally, so Lambda's Errors metric does not move. Count +# the ERROR line instead and post to javabin-alerts, so a missing IdP does not +# silently freeze every client in the pool. +resource "aws_cloudwatch_log_metric_filter" "app_auth_sync_skipped" { + name = "${var.project}-app-auth-sync-skipped" + log_group_name = "/aws/lambda/${aws_lambda_function.app_auth_sync.function_name}" + pattern = "\"client left unchanged\"" + + metric_transformation { + name = "AppAuthSyncSkipped" + namespace = "Javabin/AppAuthSync" + value = "1" + default_value = "0" + } +} + +resource "aws_cloudwatch_metric_alarm" "app_auth_sync_skipped" { + alarm_name = "${var.project}-app-auth-sync-skipped" + alarm_description = "app-auth-sync skipped an app.yaml auth request because a configured identity provider is missing from the pool. See /aws/lambda/${aws_lambda_function.app_auth_sync.function_name}." + comparison_operator = "GreaterThanThreshold" + evaluation_periods = 1 + metric_name = aws_cloudwatch_log_metric_filter.app_auth_sync_skipped.metric_transformation[0].name + namespace = aws_cloudwatch_log_metric_filter.app_auth_sync_skipped.metric_transformation[0].namespace + period = 300 + statistic = "Sum" + threshold = 0 + treat_missing_data = "notBreaching" + + alarm_actions = [var.alerts_topic_arn] +} + ################################################################################ # Plan Review — LLM risk review of app Terraform plans (javabin-plan-review) # diff --git a/terraform/platform/lambdas/variables.tf b/terraform/platform/lambdas/variables.tf index 20422ef..467cba4 100644 --- a/terraform/platform/lambdas/variables.tf +++ b/terraform/platform/lambdas/variables.tf @@ -138,6 +138,16 @@ variable "auth_memberships_table_arn" { type = string } +variable "internal_identity_providers" { + description = "Sign-in IdPs app-auth-sync puts on internal-pool app clients" + type = list(string) +} + +variable "external_identity_providers" { + description = "Sign-in IdPs app-auth-sync puts on external-pool app clients" + type = list(string) +} + variable "atlassian_sync_apply" { description = "Let team-provisioner write Atlassian group memberships; false = report the plan only" type = bool diff --git a/terraform/platform/main.tf b/terraform/platform/main.tf index 7107138..b6e8b35 100644 --- a/terraform/platform/main.tf +++ b/terraform/platform/main.tf @@ -91,6 +91,8 @@ module "lambdas" { external_user_pool_arn = module.identity.external_user_pool_arn auth_memberships_table_name = module.identity.auth_memberships_table_name auth_memberships_table_arn = module.identity.auth_memberships_table_arn + internal_identity_providers = module.identity.internal_identity_providers + external_identity_providers = module.identity.external_identity_providers identity_store_id = var.identity_store_id sso_instance_arn = var.sso_instance_arn alert_dedup_table_name = module.monitoring.alert_dedup_table_name diff --git a/terraform/platform/monitoring/main.tf b/terraform/platform/monitoring/main.tf index 4b527f8..4e736f2 100644 --- a/terraform/platform/monitoring/main.tf +++ b/terraform/platform/monitoring/main.tf @@ -50,6 +50,17 @@ resource "aws_sns_topic_policy" "alerts" { Principal = { Service = "events.amazonaws.com" } Action = "SNS:Publish" Resource = aws_sns_topic.alerts.arn + }, + { + Sid = "AllowCloudWatchAlarmsPublish" + Effect = "Allow" + Principal = { Service = "cloudwatch.amazonaws.com" } + Action = "SNS:Publish" + Resource = aws_sns_topic.alerts.arn + Condition = { + StringEquals = { "aws:SourceAccount" = var.aws_account_id } + ArnLike = { "aws:SourceArn" = "arn:aws:cloudwatch:${var.region}:${var.aws_account_id}:alarm:*" } + } } ] })