diff --git a/CLAUDE.md b/CLAUDE.md index ab9f499..03bd186 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -125,7 +125,7 @@ terraform/state/ |--------|------| | `terraform/modules/platform-data/` | Read-only data sources for shared infra | | `terraform/modules/ecr-repo/` | ECR repository with lifecycle policy | -| `terraform/modules/service-routing/` | ALB target group + listener rule + DNS | +| `terraform/modules/service-routing/` | ALB target group + listener rule + DNS; optional per-path ALB sign-in (`routing.login`) | | `terraform/modules/service-role/` | ECS task IAM role with composable policies | | `terraform/modules/ecs-service/` | ECS Fargate service | | `terraform/modules/service-bucket/` | S3 bucket with IAM policy output | @@ -169,8 +169,8 @@ terraform/state/ | `budget-enforcer` | Scales ECS services to zero when team exceeds 200% budget | | `resource-tagger` | EventBridge-triggered, auto-tags created-by + commit on new resources | | `ci-broker` | Verifies the job's GitHub OIDC token (repo from signed claims), checks repo ∈ app.yaml team, vends short-lived team role credentials | -| `app-auth-sync` | Creates/deletes Cognito app clients requested by app.yaml `auth:` (SSM request → client + credentials + allowed groups) | -| `auth-claims` | Internal pool pre-token trigger: registry groups → `cognito:groups`, enforces app `auth.groups` (in `identity/`) | +| `app-auth-sync` | Creates/deletes Cognito app clients requested by app.yaml `auth:` and `routing.login` (SSM request → client + credentials + allowed groups) | +| `auth-claims` | Pre-token trigger: registry groups → `cognito:groups` (internal pool only), enforces per-client allowed groups; runs as `auth-claims-external` on the external pool (in `identity/`) | | `plan-review` | LLM review of app Terraform plans with platform credentials; writes `risk.json` (invoked by app tf-plan) | | `app-secret-rollout` | Restarts `{team}-{app}` when one of its secrets in `/javabin/apps/` changes (ECS reads secrets only at task start) | diff --git a/docs/app-yaml-reference.md b/docs/app-yaml-reference.md index f1a16e2..e87a105 100644 --- a/docs/app-yaml-reference.md +++ b/docs/app-yaml-reference.md @@ -113,6 +113,97 @@ routing: - `host` creates a Route53 A record (in the zone matching the host's domain) pointing to the platform ALB and an ALB listener rule matching the Host header. - `priority` determines listener rule evaluation order. Must be unique across all apps. Coordinate via the registry. +#### routing.login — sign-in at the load balancer + +`login` makes the platform load balancer require sign-in before a request reaches the app. You pick, per path, which Cognito pool users sign in to and which groups may pass. The app needs no auth code, so this works for simple apps and static web pages (see [recipe](#static-pages-behind-sign-in)). It is independent of [`auth`](#auth) (where the app does OIDC itself); an app can use both. + +```yaml +routing: + host: wiki.java.no + priority: 120 + login: + - paths: ["/admin/*"] # rules are checked in this order; first match wins + groups: [styret] + - paths: ["/assets/*", "/health"] + public: true # no sign-in for these + - paths: ["/*"] # default when paths is left out + groups: [helter, team-core] + pool: internal # default +``` + +Shorthands: + +```yaml +routing: + login: internal # whole site, any user of the internal pool +# or +routing: + login: # one rule; paths defaults to ["/*"] + groups: [helter] +``` + +| Field | Default | Meaning | +|---|---|---| +| `paths` | `["/*"]` | 1-4 ALB path patterns. Case-sensitive; `*` and `?` are wildcards; max 128 chars. On protected rules, `/x/*` also protects the bare `/x` (added for you unless you list `/x` yourself), and counts toward the 4. | +| `pool` | `internal` | `internal`: Javabin heroes, signed in through Google Workspace (SAML) only. The pool has no self sign-up and its clients offer no username/password sign-in. `external`: attendees, partners and other public users. Today that means Cognito email/password sign-up; other identity providers (e.g. Google, Vipps) are not set up on that pool yet. | +| `groups` | `[]` (any pool user) | Only users in at least one of these groups get through. Internal pool: registry groups (`groups/groups.yaml` with `cognito: true`, and `team-`). External pool: native Cognito groups of the external pool. Nothing in the platform creates or fills those groups yet; until it does, an admin creates them by hand (`aws cognito-idp create-group` / `admin-add-user-to-group` on `javabin-external`), so ask the platform team. | +| `public` | `false` | Forward without sign-in. Cannot be combined with `pool` or `groups`. | + +What happens: + +- A user without a session is redirected to the pool's sign-in page, then back to the page they asked for. A user outside `groups` is refused by Cognito at sign-in ("You are not in a group that has access to this app.") and never gets a session, so the app is never reached. Where the message shows (Cognito's sign-in page or the load balancer's error page) depends on the sign-in flow. +- Every path not matched by a rule keeps working as before, without sign-in. +- Sessions last 8 hours. A registry change (e.g. removing someone from a group) takes effect at their next sign-in, so at most 8 hours later. +- An app.yaml change to a rule's `pool` or `groups`, or a reorder that gives a rule a new position, renames that rule's session cookie, so everyone signs in again and the new groups apply at once. Changing only `paths` keeps existing sessions. +- One sign-in covers all rules of the app: Cognito remembers the user, so a second rule only redirects. + +Rules and priorities: + +- Each rule becomes its own ALB listener rule at the priorities **directly below** `routing.priority`: the first rule gets `priority - 1`, the next `priority - 2`, and so on. Leave that room free: if another app already uses one of those priorities, apply fails with `PriorityInUse`. `priority` must be larger than the number of rules. Nothing checks this across apps yet, so pick a `priority` with at least 10 free numbers below it (e.g. a multiple of 20 when registering a new app); apps whose priority is right next to another app's (e.g. 100 and 101) cannot add rules without moving. +- At most 10 rules. A path may appear only once, and a rule after a `/*` rule is an error (it could never match). +- Protect `/*` and open the exceptions with `public: true`, rather than protecting a few subpaths. The load balancer matches the raw request path: case-sensitively, without decoding `%xx` and without resolving `..`. An app or framework that does any of these can be reached past a subpath rule, e.g. `/Admin`, `/%61dmin/` or `/x/../admin/` past `/admin/*`. The generator warns when no rule covers `/*`. + +What the app receives on signed-in requests (headers added by the load balancer): + +| Header | Content | +|---|---| +| `x-amzn-oidc-identity` | The user's `sub` | +| `x-amzn-oidc-data` | JWT with the user's claims (`email`, `name`, ...), signed by the load balancer (ES256). Verify it with the key at `https://public-keys.auth.elb.eu-central-1.amazonaws.com/` and check that `signer` is the platform ALB's ARN. | +| `x-amzn-oidc-accesstoken` | The Cognito access token, with the user's groups in `cognito:groups`. Verify it against the pool's issuer (`COGNITO_ISSUER_URL` if the app also uses `auth`). | + +Caveats: + +- On paths that are not protected, a client can send these headers itself. Verify the signature before trusting them anywhere. +- The load balancer has no logout endpoint. To sign out, expire the `javabin-auth-p-*` cookies and redirect to the pool's `/logout`. +- How it works: each protected rule has its own Cognito client, `-.login-`, requested through SSM (`/javabin/apps///login-/auth`) and created by the platform's `javabin-app-auth-sync` Lambda, exactly like [`auth`](#how-the-client-is-created). The first apply waits 60 seconds for the client. If it fails with `ParameterNotFound`, the client was not created in time: re-run the pipeline. + +##### Static pages behind sign-in + +There is no separate static-site hosting: serve the files from a small container and let `login` protect it. `login` protects apps that run on the platform; it cannot proxy to a site hosted elsewhere (the load balancer only forwards to its own target groups). + +```dockerfile +FROM nginxinc/nginx-unprivileged:1.27-alpine +COPY site/ /usr/share/nginx/html/ +``` + +```yaml +name: styret-docs +team: core +compute: + cpu: 256 + memory: 512 + port: 8080 + health_check: / + readonly_root_filesystem: false # nginx writes its cache and pid outside /tmp +routing: + host: styret.javabin.no + priority: 130 + login: + groups: [styret] +``` + +The health check comes from inside the VPC and is not affected by `login`. + ### resources Optional infrastructure resources. Each creates the AWS resource and auto-wires environment variables and IAM policies to the ECS task. @@ -233,7 +324,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. -- `external` — public users. +- `external` — public users. Tokens carry only native Cognito groups of the external pool; registry groups are never added there, since anyone can sign up with any email. - `none` or omitted — no Cognito integration. `auth:` with a client needs `routing.host` or explicit `callback_urls` — the defaults are derived from the host. @@ -258,7 +349,7 @@ On the very first deploy with `auth:`, the first task can start a few seconds be | Var | Source | Notes | |---|---|---| | `COGNITO_USER_POOL_ID` | Pool looked up by name | Used to construct the issuer URL on the backend | -| `COGNITO_DOMAIN` | Hosted-UI FQDN of the pool | e.g. `javabin-internal.auth.eu-central-1.amazoncognito.com`; empty for external when no custom domain is configured | +| `COGNITO_DOMAIN` | Hosted-UI FQDN of the pool | e.g. `javabin-internal.auth.eu-central-1.amazoncognito.com` (external: `javabin-external.auth.eu-central-1.amazoncognito.com`) | | `COGNITO_ISSUER_URL` | `https://cognito-idp.{region}.amazonaws.com/{pool_id}` | OIDC issuer for JWT validation | | `COGNITO_WELL_KNOWN_URL` | `{issuer}/.well-known/openid-configuration` | OIDC discovery document | | `COGNITO_CLIENT_ID` | SSM (ECS task `secrets`) | `confidential` and `public` only | @@ -352,3 +443,4 @@ All app resources are prefixed with the team name for ABAC enforcement. | CloudWatch logs | `/ecs/{team}/{name}` | | DNS record | `{routing.host}` | | SSM (Cognito) | `/javabin/apps/{team}/{name}/cognito-client-{id,secret}` | +| Cognito client (`routing.login`) | `{team}-{name}.login-{N}`, credentials under `/javabin/apps/{team}/{name}/login-{N}/` | diff --git a/docs/lambda-functions.md b/docs/lambda-functions.md index 3ee17e9..2b92893 100644 --- a/docs/lambda-functions.md +++ b/docs/lambda-functions.md @@ -125,7 +125,9 @@ 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 every pool IdP, plus `COGNITO` (username/password) on the external pool only (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. + +**Slots:** a request one level down, `/javabin/apps/{team}/{app}/login-N/auth` (written by service-routing for each `routing.login` rule), gets its own client `{team}-{app}.login-N` with credentials under `/javabin/apps/{team}/{app}/login-N/`. `.` cannot occur in app names, so slot clients never collide with another app's client. Other subpaths are ignored. **Concurrency:** reserved concurrency 1, so two changes to one app cannot race into duplicate clients. @@ -136,7 +138,9 @@ Idempotent: skips resources that already have a `created-by` tag (preserves orig **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. -**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. +**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. Registry groups are granted only for an email the pool vouches for: federated users (`cognito:user_status = EXTERNAL_PROVIDER`) or `email_verified = true`. + +**External pool:** the same code runs as `javabin-auth-claims-external` with `REGISTRY_GROUPS=false`. Anyone can sign up in the external pool with any email, so registry groups (keyed by email) are never added there; native Cognito groups still go into `cognito:groups`, and per-client `allowed_groups` are still enforced. **Lives in:** `terraform/platform/identity/` (the pool references it, so it can't sit in `lambdas/` without a module cycle). diff --git a/docs/reusable-modules.md b/docs/reusable-modules.md index e7b37fb..83dcc2f 100644 --- a/docs/reusable-modules.md +++ b/docs/reusable-modules.md @@ -38,9 +38,12 @@ ECR repository with 4-rule lifecycle policy. ALB target group + listener rule (host-header routing) + Route53 A record (alias to ALB). -**Required inputs:** `name`, `project`, `vpc_id`, `https_listener_arn`, `listener_rule_priority`, `host_header`, `route53_zone_id`, `alb_dns_name`, `alb_zone_id` +**Required inputs:** `name`, `team`, `vpc_id`, `https_listener_arn`, `listener_rule_priority`, `host_header`, `route53_zone_id`, `alb_dns_name`, `alb_zone_id` +**Optional inputs:** `project`, `login_rules` (app.yaml `routing.login`), `login_session_timeout` (default 8 h), `login_client_wait` (default `60s`), `internal_identity_provider` (default `GoogleSAML`) **Outputs:** `target_group_arn`, `target_group_arn_suffix`, `dns_name` +`login_rules` (map keyed `p1`, `p2`, ...) adds one listener rule per entry, matching host + paths at the entry's priority (below `listener_rule_priority`). Protected entries get an `authenticate-oidc` action against the chosen Cognito pool, with a per-rule client `{team}-{name}.login-N` requested through a nested `cognito-app-client` (slot `login-N`); group restrictions are enforced by Cognito (auth-claims). `public` entries forward without sign-in. The host rule `depends_on` them, so a new app serves nothing until they exist. `authenticate-cognito` is not used: it needs `cognito-idp:DescribeUserPoolClient` on the platform pools, which the developer boundary denies. Requires the `hashicorp/time` provider (for the wait on the async client creation). + ## service-role ECS task IAM role with CloudWatch Logs permissions + composable additional policies. @@ -150,8 +153,8 @@ Auto-wires: resource env vars (bucket names, table names, queue URLs), secret AR Requests a Cognito OAuth client for the app. It does **not** create the client: it writes the request to SSM `/{project}/apps/{team}/{app_name}/auth`, and the platform `app-auth-sync` Lambda creates the client `{team}-{app_name}` and writes its credentials next to the request (`cognito-client-id`, `cognito-client-secret`). See [app-yaml-reference.md#auth](app-yaml-reference.md#auth) for why. -**Inputs:** `app_name`, `team`, `pool` (internal/external), `client_type` (confidential/public/none), `callback_urls`, `logout_urls`, `allowed_oauth_scopes`, `allowed_groups`, `external_pool_custom_domain` -**Outputs:** `user_pool_id`, `user_pool_domain`, `issuer_url`, `well_known_url`, `client_id_arn`, `client_secret_arn` (SSM ARNs for ECS `secrets`), `request_parameter_name` +**Inputs:** `app_name`, `team`, `pool` (internal/external), `client_type` (confidential/public/none), `callback_urls`, `logout_urls`, `allowed_oauth_scopes`, `allowed_groups`, `external_pool_custom_domain`, `slot` (optional `login-N`: an extra client `{team}-{app_name}.login-N` with everything under `/{project}/apps/{team}/{app_name}/login-N/`) +**Outputs:** `user_pool_id`, `user_pool_domain`, `issuer_url`, `well_known_url`, `client_id_arn`, `client_secret_arn` (SSM ARNs for ECS `secrets`), `client_id_parameter_name`, `client_secret_parameter_name`, `request_parameter_name`, `request_value` (the request JSON; depends on the parameter existing) ## Policy Composition Pattern diff --git a/scripts/generate-modules.py b/scripts/generate-modules.py index b1d4562..fc7ae06 100755 --- a/scripts/generate-modules.py +++ b/scripts/generate-modules.py @@ -60,7 +60,7 @@ class AppYamlError(ValueError): "team": None, "compute": {"cpu", "memory", "port", "desired_count", "health_check", "health_check_matcher", "user", "readonly_root_filesystem"}, - "routing": {"host", "priority", "deregistration_delay"}, + "routing": {"host", "priority", "deregistration_delay", "login"}, "domain": None, "resources": {"buckets", "databases", "secrets", "queues"}, "auth": {"pool", "client", "callback_urls", "logout_urls", "scopes", "groups", "env"}, @@ -85,6 +85,15 @@ class AppYamlError(ValueError): } AUTH_CLIENT_TYPES = ("confidential", "public", "none") +AUTH_POOLS = ("internal", "external") + +# routing.login — sign-in enforced by the ALB (authenticate-oidc) per path. +LOGIN_KEYS = {"paths", "pool", "groups", "public"} +LOGIN_MAX_RULES = 10 +LOGIN_MAX_PATHS = 4 # an ALB rule takes 5 condition values; the host is one +# ALB path-pattern characters (case-sensitive, * and ? wildcards, max 128 chars) +LOGIN_PATH_RE = re.compile(r"^/[A-Za-z0-9_\-.$/~\"'@:+*?&]{0,127}$") +GROUP_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]{0,127}$") NAME_RE = re.compile(r"^[a-z][a-z0-9-]{0,19}$") TEAM_RE = re.compile(r"^[a-z]{1,12}$") @@ -110,6 +119,103 @@ def _check_keys(section, data, allowed): ) +def normalize_login(routing): + """routing.login as a validated list of {paths, pool, groups, public}. + + Accepts `login: internal|external`, a single mapping, or a list of + mappings. Rules are matched in the listed order; the first match wins. + """ + raw = routing.get("login") + if raw is None or raw is False or raw == []: + return [] + if isinstance(raw, str): + if raw not in AUTH_POOLS: + raise AppYamlError(f"routing.login: {raw!r} must be 'internal', 'external', a mapping or a list") + raw = [{"pool": raw}] + elif isinstance(raw, dict): + raw = [raw] + if not isinstance(raw, list): + raise AppYamlError("routing.login: expected 'internal', 'external', a mapping or a list of mappings") + if len(raw) > LOGIN_MAX_RULES: + raise AppYamlError(f"routing.login: at most {LOGIN_MAX_RULES} rules (got {len(raw)})") + + rules, seen, catch_all = [], set(), None + for i, entry in enumerate(raw): + where = f"routing.login[{i}]" + _check_keys(where, entry, LOGIN_KEYS) + if catch_all is not None: + raise AppYamlError(f"{where}: unreachable — routing.login[{catch_all}] already matches /*") + + paths = entry.get("paths", ["/*"]) + if isinstance(paths, str): + paths = [paths] + if not isinstance(paths, list) or not 1 <= len(paths) <= LOGIN_MAX_PATHS: + raise AppYamlError(f"{where}.paths: expected a list of 1-{LOGIN_MAX_PATHS} paths") + for path in paths: + if not isinstance(path, str) or not LOGIN_PATH_RE.match(path): + raise AppYamlError( + f"{where}.paths: {path!r} must start with / and use only ALB path characters " + "(letters, digits, _-.$/~\"'@:+& and the wildcards * ?), max 128 chars" + ) + if path in seen: + raise AppYamlError(f"{where}.paths: {path!r} is listed more than once") + seen.add(path) + + public = entry.get("public", False) + if not isinstance(public, bool): + raise AppYamlError(f"{where}.public: expected true or false") + if public and ("pool" in entry or "groups" in entry): + raise AppYamlError(f"{where}: public: true cannot be combined with pool or groups") + + pool = entry.get("pool", "internal") + if pool not in AUTH_POOLS: + raise AppYamlError(f"{where}.pool: must be 'internal' or 'external' (got {pool!r})") + + groups = entry.get("groups") or [] + if not isinstance(groups, list) or not all(isinstance(g, str) and GROUP_RE.match(g) for g in groups): + raise AppYamlError(f"{where}.groups: expected a list of group names") + + rules.append({"paths": paths, "pool": pool, "groups": groups, "public": public}) + if "/*" in paths: + catch_all = i + + # /admin/* does not match /admin itself: protect the bare prefix too, + # unless the developer listed it somewhere explicitly. + for i, rule in enumerate(rules): + if rule["public"]: + continue + bare = [p[:-2] for p in rule["paths"] if p.endswith("/*") and p != "/*" and p[:-2] not in seen] + if bare: + seen.update(bare) + rule["paths"] = rule["paths"] + bare + if len(rule["paths"]) > LOGIN_MAX_PATHS: + raise AppYamlError( + f"routing.login[{i}].paths: with the bare prefixes added ({', '.join(bare)} — /x/* does not " + f"match /x) this rule needs more than {LOGIN_MAX_PATHS} paths; split it into several entries" + ) + + priority = routing.get("priority") + if isinstance(priority, int) and priority - len(rules) < 1: + raise AppYamlError( + f"routing.priority: {priority} leaves no room for {len(rules)} login rule(s) — " + f"they use the priorities directly below it; use at least {len(rules) + 1}" + ) + return rules + + +def login_warnings(rules): + """Advice that does not fail the build.""" + protected = [r for r in rules if not r["public"]] + if protected and not any("/*" in r["paths"] for r in rules): + return [ + "routing.login protects only some paths. The load balancer matches the raw request path, " + "case-sensitively and without decoding or normalizing it, so an app that does any of that can " + "be reached past the rule (e.g. /Admin, /%61dmin or /x/../admin vs /admin/*). " + "Prefer protecting /* and listing the open paths with public: true." + ] + return [] + + def validate(app): """Raise AppYamlError if app.yaml uses unknown fields or invalid values.""" if not isinstance(app, dict): @@ -140,6 +246,7 @@ def validate(app): ) if not isinstance(routing.get("priority"), int) or not 1 <= routing["priority"] <= 50000: raise AppYamlError("routing.priority is required: a unique integer 1-50000") + normalize_login(routing) for rtype, items in (app.get("resources") or {}).items(): if not isinstance(items, list): @@ -297,7 +404,7 @@ def src(mod): # Routing (if host specified) if routing.get("host"): - blocks.append(module_block("routing", src("service-routing"), { + routing_attrs = { "name": name, "team": team, "vpc_id": Ref("module.platform.vpc_id"), @@ -311,7 +418,25 @@ def src(mod): "alb_dns_name": Ref("module.platform.alb_dns_name"), "alb_zone_id": Ref("module.platform.alb_zone_id"), "deregistration_delay": routing.get("deregistration_delay", 30), - })) + } + # routing.login: one ALB rule per entry, at the priorities directly + # below routing.priority so they are evaluated before the host rule. + login = normalize_login(routing) + if login: + routing_attrs["project"] = PROJECT + routing_attrs["login_rules"] = { + f"p{i + 1}": { + "groups": rule["groups"], + "paths": rule["paths"], + "pool": rule["pool"], + "priority": routing["priority"] - 1 - i, + "public": rule["public"], + } + for i, rule in enumerate(login) + } + for warning in login_warnings(login): + print(f"::warning file=app.yaml::{warning}") + blocks.append(module_block("routing", src("service-routing"), routing_attrs)) # Collection modules (buckets, databases, secrets, queues) policy_map = {} # name -> module output ref for access_policy_json diff --git a/scripts/tests/test_generate_modules.py b/scripts/tests/test_generate_modules.py index 6236ef1..246637b 100644 --- a/scripts/tests/test_generate_modules.py +++ b/scripts/tests/test_generate_modules.py @@ -1,6 +1,8 @@ """Tests for generate-modules.py — run: python3 -m unittest discover scripts/tests""" +import contextlib import importlib.util +import io import os import shutil import subprocess @@ -178,6 +180,127 @@ def test_localhost_callbacks_are_passed_through(self): self.assertIn('"http://localhost:3000/"', generate(app)["main.tf"]) +WIKI = { + "name": "wiki", + "team": "core", + "compute": {"port": 8080, "health_check": "/"}, + "routing": { + "host": "wiki.java.no", + "priority": 120, + "login": [ + {"paths": ["/admin/*"], "groups": ["styret"]}, + {"paths": ["/assets/*", "/health"], "public": True}, + {"groups": ["helter", "team-core"], "pool": "internal"}, + ], + }, +} + + +def routing_block(main_tf): + start = main_tf.index('module "routing"') + return main_tf[start:main_tf.index("\n}\n", start) + 2] + + +class LoginTest(unittest.TestCase): + def with_login(self, login, priority=120): + return {**WIKI, "routing": {"host": "wiki.java.no", "priority": priority, "login": login}} + + def assertRejected(self, login, fragment, priority=120): + with self.assertRaises(gen.AppYamlError) as cm: + generate(self.with_login(login, priority)) + self.assertIn(fragment, str(cm.exception)) + + def test_rules_get_priorities_directly_below_the_host_rule(self): + block = routing_block(generate(WIKI)["main.tf"]) + self.assertIn(' project = "javabin"', block) + self.assertIn(""" login_rules = { + p1 = { + groups = ["styret"] + paths = ["/admin/*", "/admin"] + pool = "internal" + priority = 119 + public = false + } + p2 = { + groups = [] + paths = ["/assets/*", "/health"] + pool = "internal" + priority = 118 + public = true + } + p3 = { + groups = ["helter", "team-core"] + paths = ["/*"] + pool = "internal" + priority = 117 + public = false + } + }""", block) + self.assertIn("listener_rule_priority = 120", block) + + def test_bare_prefix_is_protected_with_the_wildcard(self): + rules = gen.normalize_login({"priority": 120, "login": [ + {"paths": ["/admin/*", "/ops/*"], "groups": ["styret"]}, + {"paths": ["/docs/*"], "public": True}, + {"paths": ["/api/*"]}, + {"paths": ["/api"], "public": True}, + ]}) + self.assertEqual(rules[0]["paths"], ["/admin/*", "/ops/*", "/admin", "/ops"]) + self.assertEqual(rules[1]["paths"], ["/docs/*"]) # public rules are left alone + self.assertEqual(rules[2]["paths"], ["/api/*"]) # /api is listed explicitly + self.assertRejected({"paths": ["/a/*", "/b/*", "/c/*"]}, "split it into several entries") + + def test_shorthands_protect_the_whole_site(self): + for login in ("external", {"pool": "external"}, [{"pool": "external", "paths": ["/*"]}]): + block = routing_block(generate(self.with_login(login))["main.tf"]) + self.assertIn('paths = ["/*"]', block) + self.assertIn('pool = "external"', block) + self.assertIn("priority = 119", block) + + def test_apps_without_login_are_unchanged(self): + block = routing_block(generate(FROSTING)["main.tf"]) + self.assertNotIn("login_rules", block) + self.assertNotIn("project", block) + for login in (None, [], False): + self.assertEqual(generate({**FROSTING, "routing": {**FROSTING["routing"], "login": login}}), + generate(FROSTING)) + + def test_partial_protection_warns_about_case_sensitive_paths(self): + out = io.StringIO() + with contextlib.redirect_stdout(out): + generate(self.with_login([{"paths": ["/admin/*"], "groups": ["styret"]}])) + self.assertIn("::warning file=app.yaml::routing.login protects only some paths", out.getvalue()) + + out = io.StringIO() + with contextlib.redirect_stdout(out): + generate(WIKI) + self.assertEqual(out.getvalue(), "") + + def test_login_needs_a_host(self): + with self.assertRaises(gen.AppYamlError) as cm: + generate({**WIKI, "routing": {"priority": 120, "login": "internal"}}) + self.assertIn("routing.host is required", str(cm.exception)) + + def test_invalid_rules_are_rejected(self): + self.assertRejected("both", "must be 'internal', 'external'") + self.assertRejected(42, "expected 'internal', 'external', a mapping or a list") + self.assertRejected([{"paths": [f"/p{i}"]} for i in range(11)], "at most 10 rules") + self.assertRejected({"paths": ["/a", "/b", "/c", "/d", "/e"]}, "1-4 paths") + self.assertRejected({"paths": []}, "1-4 paths") + self.assertRejected({"paths": ["admin"]}, "must start with /") + self.assertRejected({"paths": ["/a b"]}, "ALB path characters") + self.assertRejected([{"paths": ["/a"]}, {"paths": ["/a"], "public": True}], "listed more than once") + self.assertRejected([{"paths": ["/*"]}, {"paths": ["/admin/*"]}], "unreachable") + self.assertRejected({"paths": ["/x"], "public": True, "groups": ["styret"]}, "cannot be combined") + self.assertRejected({"public": True, "pool": "internal"}, "cannot be combined") + self.assertRejected({"public": "yes"}, "expected true or false") + self.assertRejected({"pool": "both"}, "pool: must be 'internal' or 'external'") + self.assertRejected({"groups": "styret"}, "list of group names") + self.assertRejected({"groups": ["bad group"]}, "list of group names") + self.assertRejected({"path": ["/x"]}, "routing.login[0]: unknown field(s) path") + self.assertRejected([{"paths": ["/a"]}, {"paths": ["/b"]}], "use at least 3", priority=2) + + @unittest.skipUnless(shutil.which("terraform"), "terraform not installed") class FmtTest(unittest.TestCase): """tf-plan runs `terraform fmt -check`; generated files must pass it.""" @@ -185,7 +308,7 @@ class FmtTest(unittest.TestCase): def test_generated_files_are_fmt_clean(self): public = {**FROSTING, "auth": {"pool": "internal", "client": "public", "groups": ["helter"], "env": {"COGNITO_CLIENT_ID": "NUXT_PUBLIC_OIDC_CLIENT_ID"}}} - for app in (CUPCAKE, FROSTING, FULL, public, {"name": "worker", "team": "core"}): + for app in (CUPCAKE, FROSTING, FULL, public, WIKI, {"name": "worker", "team": "core"}): with tempfile.TemporaryDirectory() as d: for name, content in generate(app).items(): with open(os.path.join(d, name), "w") as f: diff --git a/terraform/lambda-src/app_auth_sync/handler.py b/terraform/lambda-src/app_auth_sync/handler.py index 7fa166a..eb4305f 100644 --- a/terraform/lambda-src/app_auth_sync/handler.py +++ b/terraform/lambda-src/app_auth_sync/handler.py @@ -14,6 +14,12 @@ 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. +An app can also request extra clients in a slot, one level down: +/{project}/apps/{team}/{app}/login-N/auth (service-routing writes these for +app.yaml routing.login, one per ALB sign-in rule). The client is then named +"{team}-{app}.login-N" — "." cannot occur in app names, so it never collides +with another app's client — and its credentials go next to that request. + 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. @@ -48,7 +54,13 @@ ALLOWED_SCOPES = {"openid", "email", "profile", "phone", "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$") +SLOT = r"login-[1-9][0-9]?" +REQUEST_RE = re.compile( + rf"^/{re.escape(PROJECT)}/apps/(?P[a-z]{{1,12}})/(?P[a-z][a-z0-9-]{{0,19}})(?:/(?P{SLOT}))?/auth$" +) +CRED_ID_RE = re.compile( + rf"^{re.escape(APPS_PREFIX)}(?P[a-z]+)/(?P[a-z0-9-]+)(?:/(?P{SLOT}))?/cognito-client-id$" +) ssm = boto3.client("ssm") cognito = boto3.client("cognito-idp") @@ -64,9 +76,13 @@ class InvalidRequest(ValueError): # --------------------------------------------------------------------------- def parse_name(name): - """Return (team, app) for a request parameter name, or None.""" + """Return (team, app, slot) for a request parameter name, or None. slot is "" for the main client.""" m = REQUEST_RE.match(name or "") - return (m["team"], m["app"]) if m else None + return (m["team"], m["app"], m["slot"] or "") if m else None + + +def client_name(team, app, slot=""): + return f"{team}-{app}.{slot}" if slot else f"{team}-{app}" def parse_request(raw): @@ -141,12 +157,17 @@ 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.""" +def identity_providers(user_pool_id, pool): + """Every IdP configured on the pool (e.g. GoogleSAML); plus local users on the external pool. + + The internal pool is for Javabin heroes only, who sign in through Google + Workspace. Offering COGNITO there would let anyone sign up with + username/password through the client's hosted UI. + """ 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"] + return names if pool == "internal" else names + ["COGNITO"] def client_settings(user_pool_id, client_name, req): @@ -158,7 +179,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(user_pool_id, req["pool"]), "ExplicitAuthFlows": ["ALLOW_REFRESH_TOKEN_AUTH", "ALLOW_USER_SRP_AUTH"], "PreventUserExistenceErrors": "ENABLED", } @@ -168,18 +189,18 @@ def client_settings(user_pool_id, client_name, req): # Sync / delete # --------------------------------------------------------------------------- -def _cred_names(team, app): - base = f"{APPS_PREFIX}{team}/{app}" +def _cred_names(team, app, slot=""): + base = f"{APPS_PREFIX}{team}/{app}" + (f"/{slot}" if slot else "") return f"{base}/cognito-client-id", f"{base}/cognito-client-secret" -def _put_param(name, value, secure, team, app): +def _put_param(name, value, secure, team, app, slot=""): ssm.put_parameter( Name=name, Value=value, Type="SecureString" if secure else "String", Overwrite=True, - Description=f"Managed by {PROJECT}-app-auth-sync for {team}-{app} — do not edit", + Description=f"Managed by {PROJECT}-app-auth-sync for {client_name(team, app, slot)} — do not edit", ) ssm.add_tags_to_resource( ResourceType="Parameter", @@ -192,24 +213,24 @@ def _put_param(name, value, secure, team, app): ) -def sync(team, app, req): +def sync(team, app, req, slot=""): """Make Cognito match one request. Returns the client ID.""" - client_name = f"{team}-{app}" + name = client_name(team, app, slot) # A pool switch (internal <-> external) leaves no client behind. for other in POOL_NAMES: if other != req["pool"]: other_id = pool_id(other) - stale = find_client(other_id, client_name) + stale = find_client(other_id, name) if stale: cognito.delete_user_pool_client(UserPoolId=other_id, ClientId=stale) _delete_client_groups(stale) - logger.info("Removed %s from %s pool", client_name, other) + logger.info("Removed %s from %s pool", name, other) upid = pool_id(req["pool"]) - settings = client_settings(upid, client_name, req) + settings = client_settings(upid, name, req) want_secret = req["client_type"] == "confidential" - client_id = find_client(upid, client_name) + client_id = find_client(upid, name) if client_id: has_secret = bool(cognito.describe_user_pool_client(UserPoolId=upid, ClientId=client_id) ["UserPoolClient"].get("ClientSecret")) @@ -217,27 +238,27 @@ def sync(team, app, req): # A client's secret setting is immutable; switching type means a new client cognito.delete_user_pool_client(UserPoolId=upid, ClientId=client_id) _delete_client_groups(client_id) - logger.info("Replacing %s: client_type changed to %s", client_name, req["client_type"]) + logger.info("Replacing %s: client_type changed to %s", name, req["client_type"]) client_id = None if client_id: cognito.update_user_pool_client(ClientId=client_id, **settings) - logger.info("Updated client %s (%s) in %s pool", client_name, client_id, req["pool"]) + logger.info("Updated client %s (%s) in %s pool", name, client_id, req["pool"]) else: client_id = cognito.create_user_pool_client(GenerateSecret=want_secret, **settings)["UserPoolClient"]["ClientId"] - logger.info("Created %s client %s (%s) in %s pool", req["client_type"], client_name, client_id, req["pool"]) + logger.info("Created %s client %s (%s) in %s pool", req["client_type"], name, client_id, req["pool"]) - id_param, secret_param = _cred_names(team, app) - _put_param(id_param, client_id, False, team, app) + id_param, secret_param = _cred_names(team, app, slot) + _put_param(id_param, client_id, False, team, app, slot) if want_secret: secret = cognito.describe_user_pool_client(UserPoolId=upid, ClientId=client_id)["UserPoolClient"]["ClientSecret"] - _put_param(secret_param, secret, True, team, app) + _put_param(secret_param, secret, True, team, app, slot) else: ssm.delete_parameters(Names=[secret_param]) - _put_client_groups(client_id, req["allowed_groups"], team, app) + _put_client_groups(client_id, req["allowed_groups"], name) return client_id -def _put_client_groups(client_id, groups, team, app): +def _put_client_groups(client_id, groups, name): if not MEMBERSHIPS_TABLE: if groups: # Fail closed: without the table the restriction would silently vanish @@ -248,7 +269,7 @@ def _put_client_groups(client_id, groups, team, app): Item={ "pk": {"S": f"client#{client_id}"}, "allowed_groups": {"L": [{"S": g} for g in groups]}, - "app": {"S": f"{team}-{app}"}, + "app": {"S": name}, }, ) @@ -258,17 +279,17 @@ def _delete_client_groups(client_id): dynamodb.delete_item(TableName=MEMBERSHIPS_TABLE, Key={"pk": {"S": f"client#{client_id}"}}) -def remove(team, app): +def remove(team, app, slot=""): """Delete the client from every pool and its credential parameters.""" - client_name = f"{team}-{app}" + name = client_name(team, app, slot) for pool in POOL_NAMES: upid = pool_id(pool) - client_id = find_client(upid, client_name) + client_id = find_client(upid, name) if client_id: cognito.delete_user_pool_client(UserPoolId=upid, ClientId=client_id) _delete_client_groups(client_id) - logger.info("Deleted client %s from %s pool", client_name, pool) - ssm.delete_parameters(Names=list(_cred_names(team, app))) + logger.info("Deleted client %s from %s pool", name, pool) + ssm.delete_parameters(Names=list(_cred_names(team, app, slot))) def read_request(name): @@ -283,14 +304,15 @@ def handle_change(name, operation): if not owner: logger.info("Ignoring %s: not an app auth request", name) return {"status": "ignored"} - team, app = owner + team, app, slot = owner + client = client_name(team, app, slot) # Re-read instead of trusting the event: the parameter is the source of # truth, and a stale Delete event must not remove a live client. raw = read_request(name) if raw is None: - remove(team, app) - return {"status": "deleted", "client": f"{team}-{app}"} + remove(team, app, slot) + return {"status": "deleted", "client": client} if operation == "Delete": logger.info("Delete event for %s but the parameter exists again — syncing", name) try: @@ -298,7 +320,7 @@ 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)} + return {"status": "synced", "client": client, "client_id": sync(team, app, req, slot)} def reconcile(): @@ -312,23 +334,24 @@ def reconcile(): wanted[owner] = p["Value"] results = {"synced": [], "invalid": [], "deleted": []} - for (team, app), raw in sorted(wanted.items()): + for (team, app, slot), raw in sorted(wanted.items()): + name = client_name(team, app, slot) try: - sync(team, app, parse_request(raw)) - results["synced"].append(f"{team}-{app}") + sync(team, app, parse_request(raw), slot) + results["synced"].append(name) except InvalidRequest as e: - logger.error("Invalid auth request for %s-%s: %s", team, app, e) - results["invalid"].append(f"{team}-{app}") + logger.error("Invalid auth request for %s: %s", name, e) + results["invalid"].append(name) # Orphans: clients whose credential parameter exists (so this Lambda made # them) but whose request is gone. Clients created by other means are # never touched. for page in paginator.paginate(Path=APPS_PREFIX, Recursive=True): for p in page["Parameters"]: - m = re.match(rf"^{re.escape(APPS_PREFIX)}([a-z]+)/([a-z0-9-]+)/cognito-client-id$", p["Name"]) - if m and (m[1], m[2]) not in wanted: - remove(m[1], m[2]) - results["deleted"].append(f"{m[1]}-{m[2]}") + m = CRED_ID_RE.match(p["Name"]) + if m and (m["team"], m["app"], m["slot"] or "") not in wanted: + remove(m["team"], m["app"], m["slot"] or "") + results["deleted"].append(client_name(m["team"], m["app"], m["slot"] or "")) logger.info("Reconcile: %s", json.dumps(results)) return results diff --git a/terraform/lambda-src/auth_claims/handler.py b/terraform/lambda-src/auth_claims/handler.py index b40eb76..3291fc7 100644 --- a/terraform/lambda-src/auth_claims/handler.py +++ b/terraform/lambda-src/auth_claims/handler.py @@ -15,6 +15,16 @@ refuse the sign-in, so the app never receives a token. Clients without an entry, or with an empty list, accept any pool user. + +Registry groups are granted only for an email the pool can vouch for: a +federated user (the IdP asserts the address at every sign-in) or a native user +whose email is verified. Otherwise anyone able to set their own email attribute +could claim another member's groups. + +The external pool runs the same code as javabin-auth-claims-external with +REGISTRY_GROUPS=false: anyone can sign up there with any email, so registry +membership (keyed by email) must not be granted. Only native Cognito groups +count, and the per-client allowed groups are still enforced. """ import logging @@ -28,6 +38,7 @@ TABLE = os.environ.get("MEMBERSHIPS_TABLE", "javabin-auth-memberships") CACHE_SECONDS = int(os.environ.get("CACHE_SECONDS", "60")) +REGISTRY_GROUPS = os.environ.get("REGISTRY_GROUPS", "true").lower() != "false" dynamodb = boto3.client("dynamodb") @@ -69,13 +80,25 @@ def allowed_groups(client_id): return set(_strings(item.get("allowed_groups"))) if item else set() +def trusted_email(attributes): + """The user's email if the pool vouches for it, else "".""" + email = (attributes.get("email") or "").lower() + federated = attributes.get("cognito:user_status") == "EXTERNAL_PROVIDER" + verified = str(attributes.get("email_verified", "")).lower() == "true" + return email if federated or verified else "" + + def handler(event, context): request = event.get("request", {}) - email = (request.get("userAttributes", {}).get("email") or "").lower() + attributes = request.get("userAttributes", {}) + email = (attributes.get("email") or "").lower() client_id = event.get("callerContext", {}).get("clientId", "") native = (request.get("groupConfiguration") or {}).get("groupsToOverride") or [] - registry = sorted(g for g, members in group_members().items() if email and email in members) + registry = [] + if REGISTRY_GROUPS: + trusted = trusted_email(attributes) + registry = sorted(g for g, members in group_members().items() if trusted and trusted in members) groups = sorted(set(native) | set(registry)) allowed = allowed_groups(client_id) if client_id else set() diff --git a/terraform/lambda-src/tests/test_app_auth_sync.py b/terraform/lambda-src/tests/test_app_auth_sync.py index 40ab733..faf19a1 100644 --- a/terraform/lambda-src/tests/test_app_auth_sync.py +++ b/terraform/lambda-src/tests/test_app_auth_sync.py @@ -140,7 +140,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"]) + # Internal: Google Workspace only — no username/password sign-up + 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") @@ -256,5 +257,74 @@ def test_localhost_callbacks_allowed_but_not_other_http(self): self.assertEqual(self.h.handler(event(REQ, "Update"), None)["status"], "invalid") +class LoginSlotTest(AppAuthSyncTest): + """service-routing requests one client per ALB sign-in rule, one level down.""" + SLOT_REQ = "/javabin/apps/cupcake/frosting/login-1/auth" + ALB = {"pool": "internal", "client_type": "confidential", + "callback_urls": ["https://frosting.java.no/oauth2/idpresponse"], "allowed_groups": ["styret"]} + + def test_slot_names(self): + self.assertEqual(self.h.parse_name(self.SLOT_REQ), ("cupcake", "frosting", "login-1")) + self.assertEqual(self.h.parse_name(REQ), ("cupcake", "frosting", "")) + self.assertEqual(self.h.client_name("cupcake", "frosting", "login-1"), "cupcake-frosting.login-1") + + def test_slot_client_lives_next_to_the_main_client(self): + self.put_request() + self.h.handler(event(REQ, "Create"), None) + self.put_request(self.ALB, name=self.SLOT_REQ) + result = self.h.handler(event(self.SLOT_REQ, "Create"), None) + + self.assertEqual(result["client"], "cupcake-frosting.login-1") + names = {c["ClientName"]: cid for cid, c in self.cognito.clients.items()} + self.assertEqual(set(names), {"cupcake-frosting", "cupcake-frosting.login-1"}) + slot_id = names["cupcake-frosting.login-1"] + base = "/javabin/apps/cupcake/frosting/login-1" + self.assertEqual(self.ssm.params[f"{base}/cognito-client-id"], slot_id) + self.assertEqual(self.ssm.params[f"{base}/cognito-client-secret"], self.cognito.clients[slot_id]["ClientSecret"]) + self.assertEqual(self.ssm.tags[f"{base}/cognito-client-secret"]["team"], "cupcake") + self.assertNotIn("COGNITO", self.cognito.clients[slot_id]["SupportedIdentityProviders"]) + item = self.dynamo.items[f"client#{slot_id}"] + self.assertEqual([v["S"] for v in item["allowed_groups"]["L"]], ["styret"]) + self.assertEqual(item["app"]["S"], "cupcake-frosting.login-1") + # the main client's credentials are untouched + self.assertEqual(self.ssm.params["/javabin/apps/cupcake/frosting/cognito-client-id"], names["cupcake-frosting"]) + + def test_deleting_the_slot_leaves_the_main_client(self): + self.put_request() + self.h.handler(event(REQ, "Create"), None) + self.put_request(self.ALB, name=self.SLOT_REQ) + self.h.handler(event(self.SLOT_REQ, "Create"), None) + del self.ssm.params[self.SLOT_REQ] + + self.assertEqual(self.h.handler(event(self.SLOT_REQ, "Delete"), None)["status"], "deleted") + _, client = self.only_client() + self.assertEqual(client["ClientName"], "cupcake-frosting") + self.assertFalse([k for k in self.ssm.params if "/login-1/" in k]) + self.assertIn("/javabin/apps/cupcake/frosting/cognito-client-id", self.ssm.params) + + def test_reconcile_keeps_slot_clients_and_removes_slot_orphans(self): + self.put_request(self.ALB, name=self.SLOT_REQ) + gone = "/javabin/apps/cupcake/frosting/login-2/auth" + self.put_request(self.ALB, name=gone) + self.h.handler(event(gone, "Create"), None) + del self.ssm.params[gone] + + result = self.h.handler({"action": "reconcile"}, None) + + self.assertEqual(result["synced"], ["cupcake-frosting.login-1"]) + self.assertEqual(result["deleted"], ["cupcake-frosting.login-2"]) + _, client = self.only_client() + self.assertEqual(client["ClientName"], "cupcake-frosting.login-1") + + def test_other_subpaths_are_not_requests(self): + for name in ("/javabin/apps/cupcake/frosting/admin/auth", + "/javabin/apps/cupcake/frosting/login-0/auth", + "/javabin/apps/cupcake/frosting/login-100/auth", + "/javabin/apps/cupcake/frosting/login-1/x/auth"): + self.ssm.params[name] = json.dumps(self.ALB) + self.assertEqual(self.h.handler(event(name, "Create"), None)["status"], "ignored", name) + self.assertEqual(self.cognito.clients, {}) + + if __name__ == "__main__": unittest.main() diff --git a/terraform/lambda-src/tests/test_auth_claims.py b/terraform/lambda-src/tests/test_auth_claims.py index c607e3f..e30b415 100644 --- a/terraform/lambda-src/tests/test_auth_claims.py +++ b/terraform/lambda-src/tests/test_auth_claims.py @@ -29,7 +29,10 @@ def client(cid, *groups): return {"pk": {"S": f"client#{cid}"}, "allowed_groups": {"L": [{"S": g} for g in groups]}} -def load(dynamo): +def load(dynamo, registry_groups=None): + os.environ.pop("REGISTRY_GROUPS", None) + if registry_groups is not None: + os.environ["REGISTRY_GROUPS"] = registry_groups boto3 = types.ModuleType("boto3") boto3.client = lambda svc: dynamo sys.modules["boto3"] = boto3 @@ -40,10 +43,13 @@ def load(dynamo): return mod -def event(email, client_id="frosting", native=()): +FEDERATED = {"cognito:user_status": "EXTERNAL_PROVIDER", "email_verified": "false"} + + +def event(email, client_id="frosting", native=(), attributes=FEDERATED): return { "callerContext": {"clientId": client_id}, - "request": {"userAttributes": {"email": email}, + "request": {"userAttributes": {"email": email, **attributes}, "groupConfiguration": {"groupsToOverride": list(native)}}, "response": {}, } @@ -78,11 +84,50 @@ def test_client_without_restriction_accepts_anyone(self): result = self.h.handler(event("grace.hopper@java.no", client_id="other-app"), None) self.assertEqual(groups_of(result), []) + def test_native_user_with_verified_email_gets_registry_groups(self): + attrs = {"cognito:user_status": "CONFIRMED", "email_verified": "true"} + result = self.h.handler(event("ada.lovelace@java.no", attributes=attrs), None) + self.assertEqual(groups_of(result), ["helter", "team-cupcake"]) + + def test_unverified_email_does_not_grant_registry_groups(self): + # e.g. a native user who just set email to a member's address + for attrs in ({"cognito:user_status": "CONFIRMED", "email_verified": "false"}, + {"cognito:user_status": "CONFIRMED"}, {}): + result = self.h.handler(event("ada.lovelace@java.no", client_id="other-app", attributes=attrs), None) + self.assertEqual(groups_of(result), [], attrs) + with self.assertRaises(self.h.AccessDenied): + self.h.handler(event("ada.lovelace@java.no", attributes=attrs), None) + def test_memberships_are_cached_between_invocations(self): self.h.handler(event("ada.lovelace@java.no"), None) self.h.handler(event("alan.turing@java.no"), None) self.assertEqual(self.dynamo.scans, 1) +class ExternalPoolTest(unittest.TestCase): + """javabin-auth-claims-external: anyone can sign up with any email there.""" + + def setUp(self): + self.dynamo = FakeDynamo([ + group("helter", "ada.lovelace@java.no"), + client("attendee-app", "speakers"), + ]) + self.h = load(self.dynamo, registry_groups="false") + + def tearDown(self): + os.environ.pop("REGISTRY_GROUPS", None) + + def test_registry_groups_are_not_granted_by_email(self): + result = self.h.handler(event("ada.lovelace@java.no", client_id="other-app"), None) + self.assertEqual(groups_of(result), []) + self.assertEqual(self.dynamo.scans, 0) + + def test_native_groups_still_count_for_allowed_groups(self): + result = self.h.handler(event("x@example.com", client_id="attendee-app", native=["speakers"]), None) + self.assertEqual(groups_of(result), ["speakers"]) + with self.assertRaises(self.h.AccessDenied): + self.h.handler(event("ada.lovelace@java.no", client_id="attendee-app"), None) + + if __name__ == "__main__": unittest.main() diff --git a/terraform/modules/cognito-app-client/main.tf b/terraform/modules/cognito-app-client/main.tf index 8f34e8d..386eedc 100644 --- a/terraform/modules/cognito-app-client/main.tf +++ b/terraform/modules/cognito-app-client/main.tf @@ -17,6 +17,11 @@ # Those parameters are owned by the Lambda, not by this Terraform. Their ARNs # are deterministic, so the ECS task definition can reference them up front; # the shared execution role reads //apps/* at task launch. +# +# With `slot` set (e.g. "login-1", used by service-routing for ALB sign-in), +# everything moves one level down — //apps////auth — +# and the client is named -., so an app can own several +# clients without colliding with another app's names. ################################################################################ data "aws_region" "current" {} @@ -31,13 +36,23 @@ locals { account_id = data.aws_caller_identity.current.account_id pool_id = tolist(data.aws_cognito_user_pools.this.ids)[0] - param_prefix = "/${var.project}/apps/${var.team}/${var.app_name}" + param_prefix = "/${var.project}/apps/${var.team}/${var.app_name}${var.slot == "" ? "" : "/${var.slot}"}" param_arn = "arn:aws:ssm:${local.region}:${local.account_id}:parameter${local.param_prefix}" - # Internal pool has a Cognito-managed domain (deterministic FQDN). The - # external pool's custom domain only exists when the platform has a - # certificate wired up, so the caller passes it in. - pool_domain = var.pool == "internal" ? "${var.internal_pool_name}.auth.${local.region}.amazoncognito.com" : var.external_pool_custom_domain + # Both pools have a Cognito-managed prefix domain named after the pool + # (deterministic FQDN). When the platform wires a custom domain for the + # external pool, the caller passes it in and it wins. + prefix_domain = "${var.pool == "internal" ? var.internal_pool_name : var.external_pool_name}.auth.${local.region}.amazoncognito.com" + pool_domain = var.pool == "external" && var.external_pool_custom_domain != "" ? var.external_pool_custom_domain : local.prefix_domain + + request_value = jsonencode({ + pool = var.pool + client_type = var.client_type + callback_urls = var.callback_urls + logout_urls = var.logout_urls + scopes = var.allowed_oauth_scopes + allowed_groups = var.allowed_groups + }) } # Validate-only apps (client_type = "none") only verify tokens issued to @@ -46,14 +61,7 @@ resource "aws_ssm_parameter" "auth_request" { count = var.client_type == "none" ? 0 : 1 name = "${local.param_prefix}/auth" - description = "Cognito client request for ${var.team}-${var.app_name} — read by the platform app-auth-sync Lambda" + description = "Cognito client request for ${var.team}-${var.app_name}${var.slot == "" ? "" : ".${var.slot}"} — read by the platform app-auth-sync Lambda" type = "String" - value = jsonencode({ - pool = var.pool - client_type = var.client_type - callback_urls = var.callback_urls - logout_urls = var.logout_urls - scopes = var.allowed_oauth_scopes - allowed_groups = var.allowed_groups - }) + value = local.request_value } diff --git a/terraform/modules/cognito-app-client/outputs.tf b/terraform/modules/cognito-app-client/outputs.tf index 5d4a506..dbd8325 100644 --- a/terraform/modules/cognito-app-client/outputs.tf +++ b/terraform/modules/cognito-app-client/outputs.tf @@ -4,7 +4,7 @@ output "user_pool_id" { } output "user_pool_domain" { - description = "Hosted-UI FQDN of the selected pool (empty for external without a custom domain)" + description = "Hosted-UI FQDN of the selected pool (the external custom domain when one is passed in)" value = local.pool_domain } @@ -32,3 +32,19 @@ output "request_parameter_name" { description = "SSM parameter holding the client request (empty for client_type = none)" value = one(aws_ssm_parameter.auth_request[*].name) } + +output "client_id_parameter_name" { + description = "Name of the SSM parameter the platform writes the client ID to" + value = "${local.param_prefix}/cognito-client-id" +} + +output "client_secret_parameter_name" { + description = "Name of the SSM parameter the platform writes the client secret to (confidential clients)" + value = "${local.param_prefix}/cognito-client-secret" +} + +output "request_value" { + description = "The client request JSON. Changes whenever the request does; depends on the request parameter existing." + value = local.request_value + depends_on = [aws_ssm_parameter.auth_request] +} diff --git a/terraform/modules/cognito-app-client/variables.tf b/terraform/modules/cognito-app-client/variables.tf index 1805e5a..4229974 100644 --- a/terraform/modules/cognito-app-client/variables.tf +++ b/terraform/modules/cognito-app-client/variables.tf @@ -8,6 +8,17 @@ variable "team" { type = string } +variable "slot" { + description = "Optional extra client for the same app (e.g. 'login-1' for ALB sign-in rules). Empty = the app's main client." + type = string + default = "" + + validation { + condition = can(regex("^$|^login-[1-9][0-9]?$", var.slot)) + error_message = "slot must be empty or login-<1-99>." + } +} + variable "pool" { description = "Which Cognito pool to register with: 'internal' or 'external'" type = string @@ -31,7 +42,7 @@ variable "external_pool_name" { } variable "external_pool_custom_domain" { - description = "Custom domain registered on the external pool (e.g. auth-external.javazone.no). Empty if none — COGNITO_DOMAIN is then empty for external apps." + description = "Custom domain registered on the external pool (e.g. auth-external.javazone.no). Empty = the pool's Cognito prefix domain (.auth..amazoncognito.com)." type = string default = "" } diff --git a/terraform/modules/service-routing/main.tf b/terraform/modules/service-routing/main.tf index 4b56c87..983e279 100644 --- a/terraform/modules/service-routing/main.tf +++ b/terraform/modules/service-routing/main.tf @@ -56,6 +56,177 @@ resource "aws_lb_listener_rule" "this" { values = [var.host_header] } } + + # Fail closed: on a new app nothing is served until the sign-in rules + # (evaluated first, lower priority numbers) exist. + depends_on = [aws_lb_listener_rule.login, aws_lb_listener_rule.public] +} + +################################################################################ +# Sign-in at the load balancer (app.yaml routing.login) +# +# Each protected rule gets its own Cognito client, requested through the +# cognito-app-client module (slot login-N) and created by the platform's +# app-auth-sync Lambda. Group restrictions are enforced by Cognito: the +# auth-claims trigger refuses tokens for the client when the user is in none +# of its allowed groups, so the ALB never gets a session for them. Cognito's +# hosted-UI session means a user signs in once for all rules. +# +# authenticate-oidc (not authenticate-cognito) because the latter requires +# cognito-idp:DescribeUserPoolClient on the platform's pools, which the +# developer boundary denies — it would expose every team's client secrets. +################################################################################ + +locals { + login_protected = { for k, v in var.login_rules : k => v if !v.public } + login_public = { for k, v in var.login_rules : k => v if v.public } + + # The ALB accepts any valid session cookie of the rule's name. Naming the + # cookie after the rule's slot, pool and groups means a change to any of + # them (tightened groups, a stricter rule taking over a slot after a + # reorder) invalidates existing sessions and forces a fresh sign-in. + login_cookie = { + for k, v in local.login_protected : + k => "javabin-auth-${k}-${substr(sha256(jsonencode([k, v.pool, sort(v.groups)])), 0, 10)}" + } +} + +module "login_client" { + source = "../cognito-app-client" + for_each = local.login_protected + + app_name = var.name + team = var.team + project = var.project + slot = "login-${trimprefix(each.key, "p")}" + pool = each.value.pool + client_type = "confidential" + callback_urls = ["https://${var.host_header}/oauth2/idpresponse"] + logout_urls = ["https://${var.host_header}/"] + allowed_groups = each.value.groups +} + +# The client is created asynchronously (SSM -> EventBridge -> Lambda). Wait +# before reading its credentials; re-armed only when the request changes. If +# the Lambda is slower, the read fails with ParameterNotFound — re-run. +resource "time_sleep" "login_client" { + for_each = local.login_protected + + create_duration = var.login_client_wait + + triggers = { + request = module.login_client[each.key].request_value + } +} + +data "aws_ssm_parameter" "login_client_id" { + for_each = local.login_protected + + name = module.login_client[each.key].client_id_parameter_name + depends_on = [time_sleep.login_client] +} + +data "aws_ssm_parameter" "login_client_secret" { + for_each = local.login_protected + + name = module.login_client[each.key].client_secret_parameter_name + with_decryption = true + depends_on = [time_sleep.login_client] +} + +resource "aws_lb_listener_rule" "login" { + for_each = local.login_protected + + listener_arn = var.https_listener_arn + priority = each.value.priority + + action { + type = "authenticate-oidc" + order = 1 + + authenticate_oidc { + issuer = module.login_client[each.key].issuer_url + authorization_endpoint = "https://${module.login_client[each.key].user_pool_domain}/oauth2/authorize" + token_endpoint = "https://${module.login_client[each.key].user_pool_domain}/oauth2/token" + user_info_endpoint = "https://${module.login_client[each.key].user_pool_domain}/oauth2/userInfo" + client_id = nonsensitive(data.aws_ssm_parameter.login_client_id[each.key].value) + client_secret = data.aws_ssm_parameter.login_client_secret[each.key].value + scope = "openid email profile" + session_cookie_name = local.login_cookie[each.key] + session_timeout = var.login_session_timeout + on_unauthenticated_request = "authenticate" + + # Internal pool: straight to Google Workspace, skipping the chooser + authentication_request_extra_params = ( + each.value.pool == "internal" && var.internal_identity_provider != "" + ? { identity_provider = var.internal_identity_provider } + : {} + ) + } + } + + action { + type = "forward" + order = 2 + target_group_arn = aws_lb_target_group.this.arn + } + + condition { + host_header { + values = [var.host_header] + } + } + + condition { + path_pattern { + values = each.value.paths + } + } + + tags = { + Name = "${var.team}-${var.name}-${each.key}" + } + lifecycle { + precondition { + condition = each.value.priority < var.listener_rule_priority + error_message = "A login rule's priority must be below listener_rule_priority, so it is evaluated before the host rule." + } + } +} + +# public: true — opt-out paths, forwarded without sign-in +resource "aws_lb_listener_rule" "public" { + for_each = local.login_public + + listener_arn = var.https_listener_arn + priority = each.value.priority + + action { + type = "forward" + target_group_arn = aws_lb_target_group.this.arn + } + + condition { + host_header { + values = [var.host_header] + } + } + + condition { + path_pattern { + values = each.value.paths + } + } + + tags = { + Name = "${var.team}-${var.name}-${each.key}" + } + lifecycle { + precondition { + condition = each.value.priority < var.listener_rule_priority + error_message = "A login rule's priority must be below listener_rule_priority, so it is evaluated before the host rule." + } + } } ################################################################################ diff --git a/terraform/modules/service-routing/variables.tf b/terraform/modules/service-routing/variables.tf index 6bcd508..d299298 100644 --- a/terraform/modules/service-routing/variables.tf +++ b/terraform/modules/service-routing/variables.tf @@ -71,3 +71,64 @@ variable "deregistration_delay" { type = number default = 30 } + +variable "project" { + description = "Project name for SSM parameter paths (sign-in clients)" + type = string + default = "javabin" +} + +variable "login_rules" { + description = <<-EOT + ALB sign-in rules from app.yaml routing.login, keyed p1, p2, ... Each rule + matches host + paths at its own priority (below listener_rule_priority). + Protected rules require sign-in to the pool and membership of one of + groups (empty = any pool user); public rules forward without sign-in. + EOT + type = map(object({ + priority = number + paths = list(string) + public = optional(bool, false) + pool = optional(string, "internal") + groups = optional(list(string), []) + })) + default = {} + + validation { + condition = alltrue([for k, v in var.login_rules : can(regex("^p[1-9][0-9]?$", k))]) + error_message = "login_rules keys must be p1, p2, ... (p1-p99)." + } + + validation { + condition = alltrue([for v in values(var.login_rules) : v.priority >= 1 && v.priority <= 50000]) + error_message = "login_rules priorities must be between 1 and 50000." + } + + validation { + condition = alltrue([for v in values(var.login_rules) : contains(["internal", "external"], v.pool)]) + error_message = "login_rules pool must be 'internal' or 'external'." + } + + validation { + condition = alltrue([for v in values(var.login_rules) : length(v.paths) >= 1 && length(v.paths) <= 4]) + error_message = "login_rules need 1-4 paths each (an ALB rule takes 5 condition values; the host is one)." + } +} + +variable "login_session_timeout" { + description = "Seconds an ALB sign-in session lasts. Bounds how long a removed group membership keeps working." + type = number + default = 28800 +} + +variable "login_client_wait" { + description = "How long to wait for app-auth-sync to create a sign-in client before reading its credentials" + type = string + default = "60s" +} + +variable "internal_identity_provider" { + description = "Identity provider the internal pool's sign-in goes straight to (empty = show the Cognito chooser)" + type = string + default = "GoogleSAML" +} diff --git a/terraform/modules/service-routing/versions.tf b/terraform/modules/service-routing/versions.tf new file mode 100644 index 0000000..3254b2b --- /dev/null +++ b/terraform/modules/service-routing/versions.tf @@ -0,0 +1,14 @@ +terraform { + required_version = ">= 1.5" + + required_providers { + aws = { + source = "hashicorp/aws" + version = "~> 5.0" + } + time = { + source = "hashicorp/time" + version = "~> 0.12" + } + } +} diff --git a/terraform/platform/identity/main.tf b/terraform/platform/identity/main.tf index e176986..faad193 100644 --- a/terraform/platform/identity/main.tf +++ b/terraform/platform/identity/main.tf @@ -7,6 +7,8 @@ # Identity Center lives in terraform/org/ (human-applied, not CI). ################################################################################ +data "aws_region" "current" {} + ################################################################################ # Internal Pool — Javabin heroes (@java.no accounts only) ################################################################################ @@ -24,6 +26,20 @@ resource "aws_cognito_user_pool" "internal" { auto_verified_attributes = ["email"] username_attributes = ["email"] + # Javabin heroes come in through Google Workspace (SAML). No self sign-up: + # app clients on this pool offer no username/password sign-in either + # (app-auth-sync), so this is the second lock on the same door. + admin_create_user_config { + allow_admin_create_user_only = true + } + + # A changed email stays pending until the new address is verified, so a + # user cannot claim another member's address (auth-claims grants registry + # groups by email). + user_attribute_update_settings { + attributes_require_verification_before_update = ["email"] + } + password_policy { minimum_length = 12 require_lowercase = true @@ -106,9 +122,22 @@ removed { resource "aws_cognito_user_pool" "external" { name = "${var.project}-external" + # Native groups only — see auth_claims_external below + lambda_config { + pre_token_generation_config { + lambda_arn = aws_lambda_function.auth_claims_external.arn + lambda_version = "V2_0" + } + } + auto_verified_attributes = ["email"] username_attributes = ["email"] + # A changed email stays pending until the new address is verified + user_attribute_update_settings { + attributes_require_verification_before_update = ["email"] + } + password_policy { minimum_length = 8 require_lowercase = true @@ -167,6 +196,14 @@ resource "aws_cognito_identity_provider" "external_google" { } } +# Cognito-managed prefix domain for the hosted UI. ALB sign-in from app.yaml +# routing.login and apps with auth: external point at it, so it is kept even +# when a custom domain is added below (a pool can have one of each). +resource "aws_cognito_user_pool_domain" "external_prefix" { + domain = "${var.project}-external" + user_pool_id = aws_cognito_user_pool.external.id +} + resource "aws_cognito_user_pool_domain" "external" { count = var.certificate_arn != "" ? 1 : 0 @@ -292,3 +329,33 @@ resource "aws_lambda_permission" "auth_claims_cognito" { principal = "cognito-idp.amazonaws.com" source_arn = aws_cognito_user_pool.internal.arn } + +# External pool: same code, but registry groups are never granted — anyone can +# sign up there with any email address. Native groups and per-client allowed +# groups still apply. A separate function (instead of the handler looking up +# which pool called it) keeps the pool -> Lambda dependency one-way. +resource "aws_lambda_function" "auth_claims_external" { + function_name = "${var.project}-auth-claims-external" + role = aws_iam_role.auth_claims.arn + handler = "handler.handler" + runtime = "python3.12" + timeout = 5 + memory_size = 256 + filename = data.archive_file.auth_claims.output_path + source_code_hash = data.archive_file.auth_claims.output_base64sha256 + + environment { + variables = { + MEMBERSHIPS_TABLE = aws_dynamodb_table.auth_memberships.name + REGISTRY_GROUPS = "false" + } + } +} + +resource "aws_lambda_permission" "auth_claims_external_cognito" { + statement_id = "AllowCognitoExternalPool" + action = "lambda:InvokeFunction" + function_name = aws_lambda_function.auth_claims_external.function_name + principal = "cognito-idp.amazonaws.com" + source_arn = aws_cognito_user_pool.external.arn +} diff --git a/terraform/platform/identity/outputs.tf b/terraform/platform/identity/outputs.tf index 149cdfe..2b163da 100644 --- a/terraform/platform/identity/outputs.tf +++ b/terraform/platform/identity/outputs.tf @@ -40,8 +40,8 @@ output "external_user_pool_arn" { } output "external_user_pool_domain" { - description = "Custom domain of the external Cognito user pool (empty if not configured)" - value = var.certificate_arn != "" ? "auth-external.${var.domain}" : "" + description = "Hosted-UI domain of the external Cognito user pool (custom domain if configured, else the Cognito prefix domain)" + value = var.certificate_arn != "" ? "auth-external.${var.domain}" : "${aws_cognito_user_pool_domain.external_prefix.domain}.auth.${data.aws_region.current.name}.amazoncognito.com" } output "auth_memberships_table_name" {