You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 663d247
Browse filesBrowse the repository at this point in the historyBrowse files
fix(identity): lock down the internal Cognito pool (#166)
## Why
Part of #157. The internal pool (`javabin-internal`,
`eu-central-1_Icikv3dtD`) is open to the internet today:
- `AllowAdminCreateUserOnly` is `false`, so self sign-up is on.
- app-auth-sync puts `COGNITO` on every client, so the hosted UI shows a
sign-up link.
Together, anyone can create a user in the internal pool with their own
Gmail address. An app with `auth: internal` and no `groups` accepts that
user. Nothing checks the email domain on the server side.
The `COGNITO` entry also puts a password form in front of Google. The
frosting client was hand-edited in the console to `[GoogleSAML]` to
remove the form. The 03:30 UTC reconcile undoes that edit.
## What
- **`identity/main.tf`**
- The internal pool gets `admin_create_user_config {
allow_admin_create_user_only = true }`.
- The internal `auth-claims` function gets two new env vars:
- `REQUIRED_EMAIL_DOMAIN = var.google_workspace_domain` (`java.no`);
- `FEDERATED_IDENTITY_PROVIDERS`, set from the internal IdP list.
- **New variables** `internal_identity_providers` (default
`["GoogleSAML"]`) and `external_identity_providers` (default
`["COGNITO"]`).
- They are declared in the identity module, output from it, and passed
to app-auth-sync as `INTERNAL_IDENTITY_PROVIDERS` /
`EXTERNAL_IDENTITY_PROVIDERS`.
- Validation: the internal list must not include `COGNITO` and must not
be empty.
- **app-auth-sync**
- Each client gets exactly its pool's IdP list.
- If a configured IdP doesn't exist in the pool, the request is skipped:
an `ERROR` log and `status: error` (reconcile lists it under `error`).
Nothing is created, moved or wiped, and the existing client keeps
working.
- Settings are resolved before the pool-switch cleanup, so a
misconfigured target pool can't delete the client in the old pool.
- **auth-claims**: when `REQUIRED_EMAIL_DOMAIN` is set, a token is
refused unless both of these hold:
- the email ends with `@java.no` (case-insensitive);
- the user is federated through a listed IdP (from the `identities`
attribute, or the `<Provider>_` username), or has `email_verified=true`.
The user sees "Sign in with your @java.no account." Logs mask the
address as `e***@java.no`.
- **Review follow-ups**
- app-auth-sync rejects `aws.cognito.signin.user.admin` for `pool:
internal` (status `invalid`, client not created or changed). That scope
would let a signed-in user call `UpdateUserAttributes` and point their
email at another `@java.no` user, whose registry groups auth-claims
would then grant. No live request uses it (only cupcake, with `openid
email profile`). The external pool still allows it.
- New `aws_cloudwatch_log_metric_filter` + `aws_cloudwatch_metric_alarm`
`javabin-app-auth-sync-skipped` on `client left unchanged` in
`/aws/lambda/javabin-app-auth-sync`, posting to `javabin-alerts` (Slack
via slack-alert). The `javabin-alerts` topic policy gets an
`AllowCloudWatchAlarmsPublish` statement scoped to this account's
alarms.
- `internal_identity_providers` documents that each entry is trusted to
vouch for email, so it must release only Workspace accounts (Google OIDC
needs an Internal-consent client, or an `email_verified`/`hd` check
first).
- auth-claims masks the email in the group-denial log too.
- **Docs**: `docs/lambda-functions.md` (both functions) and the
`docs/app-yaml-reference.md` auth section, which covers no password form
or sign-up, the `@java.no` rule, and how a denial arrives as
`error_description`.
## How verified
- Unit tests: `python3 -m unittest discover -s
terraform/lambda-src/tests` (79 OK) and `scripts/tests` (43 OK). New
tests cover:
- the fixed IdP list, and the reconcile reverting a console edit to the
list;
- env config, the missing-IdP skip on create, update, pool switch and
reconcile, and an empty list;
- for the domain rule: SAML users with `email_verified=false`, lookalike
domains, native users with and without a verified email, other IdPs, and
a pool without the rule.
- `terraform fmt -check -recursive` and `terraform validate` pass.
- Read-only checks against live state:
- **Users:** the pool has 4 users. All are `GoogleSAML_…`, `@java.no`,
`EXTERNAL_PROVIDER`, `email_verified=false`, identities provider
`GoogleSAML`. All 4 pass through the federated-IdP branch. This exact
shape is a unit test.
- **Auth requests:** there is one, `/javabin/apps/cupcake/cupcake/auth`
(frosting, public client, groups `[helter]`). After this change its
client gets `[GoogleSAML]`, the same as the console edit, so the
reconcile no longer reverts it. cupcake-backend is `client: none` and is
unaffected.
- **IdPs:** the internal pool has exactly one IdP, `GoogleSAML`, so the
configured list resolves. The external pool keeps `["COGNITO"]`, which
is what it gets today (it has no IdPs).
- **Plan expectation:**
- in-place updates: `aws_cognito_user_pool.internal`
(`admin_create_user_config` is not ForceNew),
`aws_lambda_function.auth_claims` (env) and
`aws_lambda_function.app_auth_sync` (env + code),
`aws_sns_topic_policy.alerts` (new statement);
- creates: the log metric filter and the alarm (the log group already
exists);
- no destroys, no pool replacement.
## Rollout & user actions
- None needed after merge. The CI apply updates the pool and both
Lambdas.
- The next auth-request change, or the 03:30 UTC reconcile, sets every
internal client to `[GoogleSAML]`. To apply it right away, run `aws
lambda invoke --function-name javabin-app-auth-sync --payload
'{"action":"reconcile"}' --cli-binary-format raw-in-base64-out
/dev/stdout`.
- Existing sessions keep working. Refresh tokens pass the domain check
because all current users qualify.
## Follow-ups
- **Interaction with #158 (`feat/alb-login`).** That PR edits the same
functions in app-auth-sync (`client_settings`, `sync`, `reconcile`) and
adds `auth_claims_external` from the same handler. The external function
doesn't set `REQUIRED_EMAIL_DOMAIN`, so it skips the domain check, which
is intended.
- Rebase on top of this PR: `client_settings()` now takes `pool` first.
- `service-routing`'s hardcoded `internal_identity_provider =
"GoogleSAML"` can be dropped once clients have a single IdP. The hosted
UI already redirects.
- **Overlap with #165.** It adds `deletion_protection = "ACTIVE"` to
both pools (raised in review here too; left to #165 to avoid a
duplicate). The change is a different attribute, so expect a trivial
text merge at most.
- **Not done here:** `user_attribute_update_settings {
attributes_require_verification_before_update = ["email"] }` on the
internal pool. Blocking the user-admin scope closes the reported path;
the pool setting would also affect how SAML attribute mapping updates
email, which can't be checked without applying, so it is left for a
separate, tested change.
- **#157 next step: switching to Google OIDC.**
- Set `internal_identity_providers = ["Google"]` once the IdP exists.
- Google users then count as federated. That is safe only with an
Internal-consent GCP client (java.no accounts only), as the design note
proposes. Otherwise, map `email_verified` and drop Google from the
trusted list.
- Bring `GoogleSAML` under Terraform with an `import` block, or remove
it after cutover.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copy file name to clipboardExpand all lines: docs/app-yaml-reference.md
+6-2Lines changed: 6 additions & 2 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -216,6 +216,10 @@ auth:
216
216
COGNITO_ISSUER_URL: NUXT_PUBLIC_OIDC_AUTHORITY
217
217
```
218
218
219
+
#### `scopes`
220
+
221
+
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.
222
+
219
223
#### `client`
220
224
221
225
| Value | For | You get |
@@ -232,7 +236,7 @@ Changing `client` between `confidential` and `public` replaces the Cognito clien
232
236
233
237
Both forms are supported and produce the same wiring.
234
238
235
-
- `internal`— Javabin heroes. Sign-in offers every identity provider on the pool (Google Workspace SAML) plus Cognito users.
239
+
- `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.
236
240
- `external`— public users.
237
241
- `none`or omitted — no Cognito integration.
238
242
@@ -271,7 +275,7 @@ On the very first deploy with `auth:`, the first task can start a few seconds be
271
275
272
276
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.
273
277
274
-
`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).
278
+
`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).
275
279
276
280
No Cognito groups are created from `app.yaml`; add groups in the registry.
Copy file name to clipboardExpand all lines: docs/lambda-functions.md
+10-2Lines changed: 10 additions & 2 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -125,7 +125,11 @@ Idempotent: skips resources that already have a `created-by` tag (preserves orig
125
125
**Trigger:** EventBridge "Parameter Store Change" for `/javabin/apps/*/*/auth` (Create/Update/Delete), plus a daily `{"action": "reconcile"}` at 03:30 UTC
126
126
**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.
127
127
128
-
**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.
128
+
**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.
129
+
130
+
**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).
**Concurrency:** reserved concurrency 1, so two changes to one app cannot race into duplicate clients.
131
135
@@ -134,7 +138,11 @@ Idempotent: skips resources that already have a `created-by` tag (preserves orig
134
138
## auth-claims
135
139
136
140
**Trigger:** Cognito pre-token-generation (V2) on the internal pool (`javabin-internal`), every sign-in and token refresh
137
-
**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.
141
+
**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.
142
+
143
+
**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 `<Provider>_` 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`).
**Data:**`javabin-auth-memberships` (DynamoDB). `group#<name>` items (members' emails) are written by team-provisioner on registry merges; `client#<client_id>` items (`allowed_groups`) by app-auth-sync. Memberships are cached for 60 s per container.
0 commit comments