Skip to content

feat(auth): gated Google OIDC IdP, auth-status, URL validation and auth doctor - #167

Open
Alexanderamiri wants to merge 3 commits into
mainfrom
feat/google-oidc
Open

Alexanderamiri wants to merge 3 commits into
mainfrom
feat/google-oidc

Conversation

@Alexanderamiri

@Alexanderamiri Alexanderamiri commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

Why

Part of #157. Internal-pool sign-in goes through the GoogleSAML app, which always answers for the browser's active Google account. A personal Gmail gets a dead-end 403 app_not_configured_for_user page with no account chooser. The classic hosted UI drops prompt=, so Cognito can't ask SAML for one either. The fix is a Google OIDC IdP, with an Internal-consent OAuth client and prompt=select_account.

The investigation also found that teams can't see why sign-in fails, and can't recover:

  • invalid requests and console drift are visible only in CloudWatch;
  • a replaced client leaves running tasks on a deleted client ID;
  • callback typos show up only as redirect_mismatch in the browser;
  • an auth.groups typo denies everyone.

Merge #166 (internal-pool lockdown) first. This branch is rebased on it and reuses its per-pool IdP list, so until #166 merges this PR's diff and plan also contain #166's two commits. After #166 lands, GitHub shows only the last commit.

This PR is safe to merge before the Google credentials exist. Every Google behaviour change is gated, and with #166 the internal default is ["GoogleSAML"] (what cupcake's client has live today).

What

identity (Terraform)

  • aws_cognito_identity_provider.internal_google is now gated on SSM /javabin/platform/google-oauth/internal/{client-id,client-secret}, read with aws_ssm_parameters_by_path, instead of a tfvar. Its count stays 0 until both exist.
    • authorize_scopes = "openid email profile", and mapping email, email_verified, name, username=sub.
    • authorize_url = …/v2/auth?hd=java.no&prompt=select_account. You can turn this off with google_authorize_url_params = false for the managed-login fallback.
  • Uses fix(identity): lock down the internal Cognito pool #166's per-pool lists internal_identity_providers (default ["GoogleSAML"], COGNITO refused) and external_identity_providers (["COGNITO"]), env INTERNAL_IDENTITY_PROVIDERS/EXTERNAL_IDENTITY_PROVIDERS. The same internal list is auth-claims' FEDERATED_IDENTITY_PROVIDERS, so the cutover changes clients and the trust list together.
    • Adding the Google IdP changes no client until this list names it.
    • An output precondition fails the plan if Google is listed without the SSM credentials or with an empty google_workspace_domain (auth-claims' REQUIRED_EMAIL_DOMAIN).
  • auth-claims: IdPs in EMAIL_VERIFIED_IDENTITY_PROVIDERS (default Google) are not trusted on the IdP name alone; their users also need email_verified=true, mapped from Google's claim.
  • The external pool keeps its google_client_id tfvar path. It is unchanged and still empty.

app-auth-sync

  • cognito-client-id, -secret and the new -identity-provider are written only when the value changes.
    • The secret is not decrypted. It changes only with a new client ID.
    • The client ID is written last, so restarted tasks find the matching secret.
  • Writes /javabin/apps/{team}/{app}/auth-status (String JSON, always valid and under 4096 bytes: list fields are dropped, never sliced). Fields: status (synced/invalid/error), error, client_id, client_id_changed_at (carried over between syncs), identity_providers, pool_domain, URLs, drift_corrected (fields a reconcile reverted; also logged as WARN drift), unknown_groups/empty_groups (checked against group# items in the memberships table), request_version, synced_at.
  • A declared IdP missing from the pool is fix(identity): lock down the internal Cognito pool #166's PoolMisconfigured skip: status error in auth-status, the client is left as it is.
  • Reconcile no longer stops at a failing app, and returns normally instead of raising (an async invoke would retry the whole run twice). The failure is logged as Sync failed for …, which fix(identity): lock down the internal Cognito pool #166's javabin-app-auth-sync-skipped metric filter now also counts.
  • Callback/logout URLs with a #fragment are rejected by the Lambda too.
  • IAM: dynamodb:GetItem on the memberships table, plus write access to */cognito-client-identity-provider and */auth-status.

Team access to auth-status

  • Teams read it through their existing /javabin/apps/{team}/* read grant.
  • Writes are denied for auth-status in the team-provisioner permission-set policy (CI-applied), and in terraform/org/identity-center.tf (human-applied; see user actions).

app-secret-rollout

  • cognito-client-id and cognito-client-identity-provider are no longer ignored. A replaced client (case 9/15) or an IdP cutover now restarts the service.
  • auth-status and cognito-client-secret are ignored: auth-status is rewritten on every sync, and a new secret always comes with a new ID written right after it, so restarting on the secret would deploy once with the old ID.
  • There is no restart loop, because unchanged values are never rewritten.

generate-modules.py

  • Callback/logout URLs must:
    • use https, except http://localhost / 127.0.0.1;
    • have no #fragment;
    • have a host equal to routing.host or listed in the new optional auth.extra_hosts.
  • client: none combined with callback_urls, logout_urls, scopes or extra_hosts is rejected; these were silently ignored before.
  • COGNITO_IDENTITY_PROVIDER is injected (an ECS secret from module.auth.identity_provider_arn) and can be aliased through auth.env.
  • Auth resolution is factored into resolve_auth(), which the doctor reuses.
  • Both live apps (frosting, cupcake-backend) still generate. Tests use their verbatim app.yaml.

cognito-app-client module

  • New outputs identity_provider_arn and auth_status_parameter_name.

scripts/auth-doctor.py

  • Uses team credentials only: app.yaml, the app's own SSM parameters, and unauthenticated HTTP.
  • Checks:
    • the request matches app.yaml;
    • auth-status is synced for the current request version, and its client ID matches cognito-client-id;
    • drift and unknown/empty groups are reported;
    • /oauth2/authorize for every callback URL redirects to the IdP, not /error?error=redirect_mismatch (it warns if Google isn't given prompt=select_account);
    • /logout for every logout URL redirects back (a mismatch lands on the hosted UI /login, verified live);
    • best effort, WARN only: the service's PRIMARY-deployment tasks started after client_id_changed_at (skipped until auth-status records one; cognito-client-id's LastModifiedDate is useless because the old code rewrote it on every reconcile).
  • A non-JSON auth-status is a FAIL, not a traceback. When auth-status is missing altogether, the doctor waits at most 30 s instead of the full --wait.
  • --json is the hook for a future javabin auth doctor. javabin-cli is not changed here.
  • tf-apply.yml gets a non-blocking Auth doctor step (continue-on-error, --wait 180, skipped for apps without auth).

platform-ci

  • After apply, scripts/reconcile-app-auth.sh queues an async reconcile. This writes the new cognito-client-identity-provider and auth-status parameters right away, so an app redeploy that picks up the new COGNITO_IDENTITY_PROVIDER secret doesn't hit ParameterNotFound. It is non-blocking, and the 03:30 reconcile is the backstop.

Docs

  • docs/cognito-google-setup.md is rewritten:
    • exact GCP steps (Internal consent screen, a separate Web OAuth client, redirect https://javabin-internal.auth.eu-central-1.amazoncognito.com/oauth2/idpresponse; the old doc listed a nonexistent auth-internal.javazone.no);
    • SSM commands;
    • the post-apply curl probe and the managed-login fallback;
    • cutover and user plan.
  • docs/cognito-saml-setup.md is marked superseded. It also noted that its groups mapping never existed.
  • docs/app-yaml-reference.md: IdP list, URL rules and extra_hosts, the /logout exact-match rule, group denial (error_description), COGNITO_IDENTITY_PROVIDER, auth-status and the doctor.
  • docs/lambda-functions.md, docs/reusable-modules.md and CLAUDE.md are updated.

How verified

  • python3 -m unittest discover -s terraform/lambda-src/tests passes (111, including fix(identity): lock down the internal Cognito pool #166's), and -s scripts/tests passes (67, including the fmt check of generated TF). New tests cover:
    • no-op syncs write only auth-status;
    • secret written before ID on replacement;
    • status content: invalid, error, drift vs request change, unknown/empty groups;
    • declared IdP list, cutover switching the IdP param without touching the client ID, a missing IdP failing cleanly;
    • reconcile continuing past a failing app;
    • rollout restarting on cognito-client-*;
    • URL validation;
    • doctor scenarios, including non-JSON status, the short wait for a missing status, and draining/stale tasks;
    • auth-status size limit (bytes, valid JSON), client_id_changed_at carry-over, fragments rejected;
    • auth-claims refusing Google users without email_verified=true.
  • terraform fmt -recursive -check and terraform validate (platform root) pass.
  • Live, read-only: the doctor against frosting's app.yaml.
    • Request check passes. auth-status doesn't exist yet, as expected before deploy.
    • Probes against the live cupcake-cupcake client: callback → GoogleSAML ✔, /nope → redirect_mismatch ✔, logout exact ✔, logout without trailing slash → hosted UI /login (this is why the logout check looks for "redirects back").
  • Live pool state checked: 4 users, all GoogleSAML_* external-provider users, no native users. /javabin/apps/* has only cupcake's auth and cognito-client-id.

Rollout & user actions

Merging (after #166) changes nothing about Google sign-in:

Expected plan: the aws_ssm_parameters_by_path read, and in-place updates to the app-auth-sync role policy, function env/code, app-secret-rollout code and team-provisioner code. No destroys.

User actions:

  1. Human apply terraform/org (identity-center.tf). It adds auth-status to the javabin-developer permission set's platform-managed deny. Per-team permission sets get it from team-provisioner on its next run.
  2. GCP (when ready): create the Internal-consent OAuth client and store it in SSM, following docs/cognito-google-setup.md §1–2. Then re-run platform CI on main. The plan creates the Google IdP, and clients are untouched.
  3. Verify: run the §3 curl probe (or the doctor after cutover) to confirm Cognito keeps prompt=select_account. If it doesn't, use the documented managed-login fallback.
  4. Cutover (separate PR): internal_identity_providers = ["Google"]. fix(identity): lock down the internal Cognito pool #166 (server-side domain check, no self sign-up) is a hard prerequisite, which stacking on it guarantees.
    • Users get a new sub. Platform memberships and groups are keyed by email, so nothing to migrate there; cupcake should check for sub-keyed data.
    • Delete the 4 GoogleSAML_* users and the GoogleSAML IdP a week later, and turn off the Google Admin SAML app.

Interactions

Follow-ups

  • The developer boundary (team CI roles, gate-vended apply credentials) doesn't deny writing auth-status/cognito-client-*. It is at 5,946/6,144 bytes and feat(platform): app-to-app calls with private networking and scopes #156 adds to it, so I left it alone; the limitation is now documented in docs/lambda-functions.md and docs/app-yaml-reference.md. The impact stays inside the team's own app: a forged status, or one extra restart before the next sync puts a credential back. A Deny in the team CI role policies is the follow-up if we want it enforced.
  • Drop ALLOW_USER_SRP_AUTH from internal clients. With fix(identity): lock down the internal Cognito pool #166 there is no COGNITO and no self sign-up, so it is unused there, but changing client auth flows is fix(identity): lock down the internal Cognito pool #166-adjacent scope.
  • javabin auth doctor in javabin-cli, wrapping scripts/auth-doctor.py --json.
  • Doctor check that the task definition contains every auth.env alias; canary-client probe in platform CI after identity applies.
  • auth.accept_clients for client: none backends (case 13).

🤖 Generated with Claude Code

- Disable self sign-up on the internal pool (allow_admin_create_user_only).
- app-auth-sync: give each pool a fixed IdP list from Terraform instead of
  every pool IdP plus COGNITO; skip and log when a configured IdP is missing.
- auth-claims: on the internal pool, refuse tokens unless the email is
  @java.no and the user is federated (GoogleSAML) or email_verified.

Part of #157

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@Alexanderamiri
Alexanderamiri requested a review from a team as a code owner October 24, 2026 14:16
@github-actions

Copy link
Copy Markdown

Terraform Plan

🚧 Changes detected — Plan: 0 to add, 4 to change, 0 to destroy.

Plan output
Acquiring state lock. This may take a few moments...
aws_ssm_parameter.atlassian_org_id: Refreshing state... [id=/javabin/platform/atlassian-org-id]
aws_ssm_parameter.atlassian_api_key: Refreshing state... [id=/javabin/platform/atlassian-api-key]

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.lambdas.aws_iam_role_policy.app_auth_sync will be updated in-place
  ~ resource "aws_iam_role_policy" "app_auth_sync" {
        id     = "javabin-app-auth-sync:javabin-app-auth-sync"
        name   = "javabin-app-auth-sync"
      ~ policy = jsonencode(
          ~ {
              ~ Statement = [
                    # (2 unchanged elements hidden)
                    {
                        Action   = [
                            "ssm:GetParameter",
                            "ssm:GetParametersByPath",
                        ]
                        Effect   = "Allow"
                        Resource = [
                            "arn:aws:ssm:eu-central-1:553637109631:parameter/javabin/apps",
                            "arn:aws:ssm:eu-central-1:553637109631:parameter/javabin/apps/*",
                        ]
                        Sid      = "ReadAuthRequests"
                    },
                  ~ {
                      ~ Action   = [
                            # (1 unchanged element hidden)
                            "dynamodb:DeleteItem",
                          + "dynamodb:GetItem",
                        ]
                        # (3 unchanged attributes hidden)
                    },
                  ~ {
                      ~ Resource = [
                            # (1 unchanged element hidden)
                            "arn:aws:ssm:eu-central-1:553637109631:parameter/javabin/apps/*/*/cognito-client-secret",
                          + "arn:aws:ssm:eu-central-1:553637109631:parameter/javabin/apps/*/*/cognito-client-identity-provider",
                          + "arn:aws:ssm:eu-central-1:553637109631:parameter/javabin/apps/*/*/auth-status",
                        ]
                        # (3 unchanged attributes hidden)
                    },
                ]
                # (1 unchanged attribute hidden)
            }
        )
        # (1 unchanged attribute hidden)
    }

  # module.lambdas.aws_lambda_function.app_auth_sync will be updated in-place
  ~ resource "aws_lambda_function" "app_auth_sync" {
        id                             = "javabin-app-auth-sync"
      ~ last_modified                  = "2026-10-23T15:31:34.000+0000" -> (known after apply)
      ~ source_code_hash               = "sIha3Wb5dyMLBep00WQdQ/yhswtf4SXSuDzJX076WoE=" -> "viuxGtq/186pkMEVGH7SGxhDSFYVYUQ/+Bvg9KwoVzg="
        tags                           = {}
        # (21 unchanged attributes hidden)

      ~ environment {
          ~ variables = {
              + "EXTERNAL_IDPS"        = "COGNITO"
              + "EXTERNAL_POOL_DOMAIN" = ""
              + "INTERNAL_IDPS"        = "GoogleSAML,COGNITO"
              + "INTERNAL_POOL_DOMAIN" = "javabin-internal.auth.eu-central-1.amazoncognito.com"
                # (2 unchanged elements hidden)
            }
        }

        # (3 unchanged blocks hidden)
    }

  # module.lambdas.aws_lambda_function.app_secret_rollout will be updated in-place
  ~ resource "aws_lambda_function" "app_secret_rollout" {
        id                             = "javabin-app-secret-rollout"
      ~ last_modified                  = "2026-10-23T22:11:34.519+0000" -> (known after apply)
      ~ source_code_hash               = "Z+R/0D8XkgfnD5R4woPCl4cGzPFa2VcQlnZkX1wB0e0=" -> "PhLlVyq1X+wa90O1EkjljJigoXfPYRksDzqjPbxjkQw="
        tags                           = {}
        # (21 unchanged attributes hidden)

        # (4 unchanged blocks hidden)
    }

  # module.lambdas.aws_lambda_function.team_provisioner will be updated in-place
  ~ resource "aws_lambda_function" "team_provisioner" {
        id                             = "javabin-team-provisioner"
      ~ last_modified                  = "2026-10-24T10:18:00.000+0000" -> (known after apply)
      ~ source_code_hash               = "FAeZVoTR17JeVhPKvfbv/DFKxW9BcEfhSFcdISb7rKo=" -> "aYQzKy+h6iDaYdx5FBgf5WSopODSbtOVe9GZqXrgPBA="
        tags                           = {}
        # (21 unchanged attributes hidden)

        # (4 unchanged blocks hidden)
    }

Plan: 0 to add, 4 to change, 0 to destroy.

─────────────────────────────────────────────────────────────────────────────

Saved the plan to: tfplan

To perform exactly these actions, run the following command to apply:
    terraform apply "tfplan"

LLM Review

Risk: 🟡 MEDIUM

Plan updates 4 existing resources in place (0 add, 0 destroy): an IAM role policy for app-auth-sync gains DynamoDB GetItem and two SSM parameter read paths, and three Lambda functions get new code and, for app-auth-sync, new environment variables.

  • 🔒 [security] The app_auth_sync IAM policy adds dynamodb:GetItem and SSM read access to cognito-client-secret, cognito-client-identity-provider and auth-status under javabin/apps//. These are Allow statements (existing Sid ReadAuthRequests is an Allow and the new Resource entries are added to existing statements). The new grants expose Cognito client secrets to the function. Confirm the wildcard /apps// scope is intended and that only the auth-sync role needs these secrets.
  • 🔒 [security] The app_auth_sync Lambda gains environment variables EXTERNAL_IDPS=COGNITO, INTERNAL_IDPS=GoogleSAML,COGNITO and INTERNAL_POOL_DOMAIN pointing to a Cognito domain. EXTERNAL_POOL_DOMAIN is set to an empty string, which may be an unintended misconfiguration. Verify the identity provider configuration is correct.
  • ✅ [routine] source_code_hash changes on all three Lambda functions (app_auth_sync, app_secret_rollout, team_provisioner), meaning new deployment packages. Routine code updates; no resource replacement.
  • 💥 [destruction] No resources are created or destroyed (Plan: 0 to add, 4 to change, 0 to destroy). All changes are in-place, so there is no data-loss risk.
  • 💰 [cost] No new billable resources are introduced. Lambda and DynamoDB usage may change marginally with the new code and extra DynamoDB reads.

…on skipped syncs

Review follow-up for the internal pool lockdown:
- app-auth-sync rejects aws.cognito.signin.user.admin for pool=internal, so a
  signed-in user cannot rewrite the email auth-claims trusts for group lookup
- alarm javabin-app-auth-sync-skipped (log metric filter) posts to
  javabin-alerts when a request is skipped for a missing IdP; the alerts topic
  policy now lets CloudWatch alarms in this account publish
- document that every internal_identity_providers entry vouches for email
- auth-claims masks the email in the group denial log too; tidy imports

Part of #157

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@Alexanderamiri

Copy link
Copy Markdown
Member Author

Notes on the risk review (MEDIUM):

  • The two new SSM ARNs go into WriteClientCredentials (PutParameter, DeleteParameter(s), AddTagsToResource), not into a read statement. The Lambda can already read /javabin/apps/* through ReadAuthRequests; that grant is unchanged. dynamodb:GetItem lets it check that group#<name> exists, for unknown_groups.
  • EXTERNAL_POOL_DOMAIN = "" is expected. The external pool has no hosted-UI domain until a certificate is wired up, so the doctor skips its HTTP probes. feat/alb-login adds a prefix domain.
  • INTERNAL_IDPS = GoogleSAML,COGNITO is exactly what every internal client has today, so the reconcile changes no client.
  • The Google IdP is not in the plan because its SSM credentials don't exist yet. That is by design.

…th doctor

Builds on the internal-pool lockdown (#166) and reuses its per-pool IdP
list (internal_identity_providers, INTERNAL_IDENTITY_PROVIDERS,
identity_providers(pool, upid), PoolMisconfigured), so one list drives
both the app clients and auth-claims' trusted IdPs.

- identity: Google OIDC IdP on the internal pool, created only once its
  OAuth client is in SSM (/javabin/platform/google-oauth/internal/*);
  authorize_url forces the account chooser (hd=java.no&prompt=select_account),
  email_verified mapped. Precondition: "Google" in
  internal_identity_providers needs the credentials and
  google_workspace_domain.
- auth-claims: Google users also need email_verified=true
  (EMAIL_VERIFIED_IDENTITY_PROVIDERS, default Google).
- app-auth-sync: write cognito-client-* only on change, add
  cognito-client-identity-provider, publish team-readable auth-status
  (status, client, client_id_changed_at, IdPs, drift corrected, unknown
  groups; always valid JSON under 4096 bytes); reject URL fragments;
  reconcile keeps going past a failing app and returns normally (logged
  for the skipped alarm) instead of raising into async retries.
- app-secret-rollout: restart on cognito-client-id / -identity-provider
  changes; ignore auth-status and cognito-client-secret (a new secret
  always comes with a new ID).
- generate-modules: validate callback/logout hosts (routing.host or
  auth.extra_hosts), https except localhost, no fragments; reject
  client: none with URLs/scopes; inject COGNITO_IDENTITY_PROVIDER.
- scripts/auth-doctor.py, run non-blocking after app apply (FAIL on a
  non-JSON status, short wait when auth-status is missing, task check is
  a WARN against client_id_changed_at for PRIMARY tasks only); platform
  CI queues a reconcile after apply.
- Docs: GCP setup with the correct redirect URI, cutover plan with the
  lockdown as a hard prerequisite; team CI roles can still write
  platform-managed parameters (documented); SAML doc marked superseded.

Part of #157

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@github-actions

Copy link
Copy Markdown

Terraform Plan

🚧 Changes detected — Plan: 2 to add, 7 to change, 0 to destroy.

Plan output
Acquiring state lock. This may take a few moments...
aws_ssm_parameter.atlassian_org_id: Refreshing state... [id=/javabin/platform/atlassian-org-id]
aws_ssm_parameter.atlassian_api_key: Refreshing state... [id=/javabin/platform/atlassian-api-key]

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  + create
  ~ update in-place

Terraform will perform the following actions:

  # module.identity.aws_cognito_user_pool.internal will be updated in-place
  ~ resource "aws_cognito_user_pool" "internal" {
        id                        = "eu-central-1_Icikv3dtD"
        name                      = "javabin-internal"
        tags                      = {
            "Name" = "javabin-internal"
        }
        # (12 unchanged attributes hidden)

      ~ admin_create_user_config {
          ~ allow_admin_create_user_only = false -> true
        }

        # (7 unchanged blocks hidden)
    }

  # module.identity.aws_lambda_function.auth_claims will be updated in-place
  ~ resource "aws_lambda_function" "auth_claims" {
        id                             = "javabin-auth-claims"
      ~ last_modified                  = "2026-10-23T15:36:35.282+0000" -> (known after apply)
      ~ source_code_hash               = "T0XK8BQ05sxP5MVCsIGH+JuY++sBeQohaGTkvMdm81M=" -> "UiryqEb37fcCA+cksNAAhDqj5PANMRxRo4q0MPfEGTo="
        tags                           = {}
        # (21 unchanged attributes hidden)

      ~ environment {
          ~ variables = {
              + "FEDERATED_IDENTITY_PROVIDERS" = "GoogleSAML"
              + "REQUIRED_EMAIL_DOMAIN"        = "java.no"
                # (1 unchanged element hidden)
            }
        }

        # (3 unchanged blocks hidden)
    }

  # module.lambdas.aws_cloudwatch_log_metric_filter.app_auth_sync_skipped will be created
  + resource "aws_cloudwatch_log_metric_filter" "app_auth_sync_skipped" {
      + id             = (known after apply)
      + log_group_name = "/aws/lambda/javabin-app-auth-sync"
      + name           = "javabin-app-auth-sync-skipped"
      + pattern        = "?\"client left unchanged\" ?\"Sync failed for\""

      + metric_transformation {
          + default_value = "0"
          + name          = "AppAuthSyncSkipped"
          + namespace     = "Javabin/AppAuthSync"
          + unit          = "None"
          + value         = "1"
        }
    }

  # module.lambdas.aws_cloudwatch_metric_alarm.app_auth_sync_skipped will be created
  + resource "aws_cloudwatch_metric_alarm" "app_auth_sync_skipped" {
      + actions_enabled                       = true
      + alarm_actions                         = [
          + "arn:aws:sns:eu-central-1:553637109631:javabin-alerts",
        ]
      + alarm_description                     = "app-auth-sync skipped an app.yaml auth request (a configured identity provider is missing from the pool) or reconcile could not sync an app. See /aws/lambda/javabin-app-auth-sync."
      + alarm_name                            = "javabin-app-auth-sync-skipped"
      + arn                                   = (known after apply)
      + comparison_operator                   = "GreaterThanThreshold"
      + evaluate_low_sample_count_percentiles = (known after apply)
      + evaluation_periods                    = 1
      + id                                    = (known after apply)
      + metric_name                           = "AppAuthSyncSkipped"
      + namespace                             = "Javabin/AppAuthSync"
      + period                                = 300
      + statistic                             = "Sum"
      + tags_all                              = {
          + "environment" = "production"
          + "managed-by"  = "terraform"
          + "repo"        = "javaBin/platform"
          + "service"     = "platform"
          + "team"        = "platform"
        }
      + threshold                             = 0
      + treat_missing_data                    = "notBreaching"
    }

  # module.lambdas.aws_iam_role_policy.app_auth_sync will be updated in-place
  ~ resource "aws_iam_role_policy" "app_auth_sync" {
        id     = "javabin-app-auth-sync:javabin-app-auth-sync"
        name   = "javabin-app-auth-sync"
      ~ policy = jsonencode(
          ~ {
              ~ Statement = [
                    # (2 unchanged elements hidden)
                    {
                        Action   = [
                            "ssm:GetParameter",
                            "ssm:GetParametersByPath",
                        ]
                        Effect   = "Allow"
                        Resource = [
                            "arn:aws:ssm:eu-central-1:553637109631:parameter/javabin/apps",
                            "arn:aws:ssm:eu-central-1:553637109631:parameter/javabin/apps/*",
                        ]
                        Sid      = "ReadAuthRequests"
                    },
                  ~ {
                      ~ Action   = [
                            # (1 unchanged element hidden)
                            "dynamodb:DeleteItem",
                          + "dynamodb:GetItem",
                        ]
                        # (3 unchanged attributes hidden)
                    },
                  ~ {
                      ~ Resource = [
                            # (1 unchanged element hidden)
                            "arn:aws:ssm:eu-central-1:553637109631:parameter/javabin/apps/*/*/cognito-client-secret",
                          + "arn:aws:ssm:eu-central-1:553637109631:parameter/javabin/apps/*/*/cognito-client-identity-provider",
                          + "arn:aws:ssm:eu-central-1:553637109631:parameter/javabin/apps/*/*/auth-status",
                        ]
                        # (3 unchanged attributes hidden)
                    },
                ]
                # (1 unchanged attribute hidden)
            }
        )
        # (1 unchanged attribute hidden)
    }

  # module.lambdas.aws_lambda_function.app_auth_sync will be updated in-place
  ~ resource "aws_lambda_function" "app_auth_sync" {
        id                             = "javabin-app-auth-sync"
      ~ last_modified                  = "2026-10-23T15:31:34.000+0000" -> (known after apply)
      ~ source_code_hash               = "sIha3Wb5dyMLBep00WQdQ/yhswtf4SXSuDzJX076WoE=" -> "3SpcZQdbYI3uJgg8wTKAnenPMYfvWhFmGF+meQoSYrQ="
        tags                           = {}
        # (21 unchanged attributes hidden)

      ~ environment {
          ~ variables = {
              + "EXTERNAL_IDENTITY_PROVIDERS" = "COGNITO"
              + "EXTERNAL_POOL_DOMAIN"        = ""
              + "INTERNAL_IDENTITY_PROVIDERS" = "GoogleSAML"
              + "INTERNAL_POOL_DOMAIN"        = "javabin-internal.auth.eu-central-1.amazoncognito.com"
                # (2 unchanged elements hidden)
            }
        }

        # (3 unchanged blocks hidden)
    }

  # module.lambdas.aws_lambda_function.app_secret_rollout will be updated in-place
  ~ resource "aws_lambda_function" "app_secret_rollout" {
        id                             = "javabin-app-secret-rollout"
      ~ last_modified                  = "2026-10-23T22:11:34.519+0000" -> (known after apply)
      ~ source_code_hash               = "Z+R/0D8XkgfnD5R4woPCl4cGzPFa2VcQlnZkX1wB0e0=" -> "IIeAoHH+LEqFBOPwakogwUQGHBkHIBEugTSRdS+r/EI="
        tags                           = {}
        # (21 unchanged attributes hidden)

        # (4 unchanged blocks hidden)
    }

  # module.lambdas.aws_lambda_function.team_provisioner will be updated in-place
  ~ resource "aws_lambda_function" "team_provisioner" {
        id                             = "javabin-team-provisioner"
      ~ last_modified                  = "2026-10-24T10:18:00.000+0000" -> (known after apply)
      ~ source_code_hash               = "FAeZVoTR17JeVhPKvfbv/DFKxW9BcEfhSFcdISb7rKo=" -> "aYQzKy+h6iDaYdx5FBgf5WSopODSbtOVe9GZqXrgPBA="
        tags                           = {}
        # (21 unchanged attributes hidden)

        # (4 unchanged blocks hidden)
    }

  # module.monitoring.aws_sns_topic_policy.alerts will be updated in-place
  ~ resource "aws_sns_topic_policy" "alerts" {
        id     = "arn:aws:sns:eu-central-1:553637109631:javabin-alerts"
      ~ policy = jsonencode(
          ~ {
              ~ Statement = [
                    # (2 unchanged elements hidden)
                    {
                        Action    = "SNS:Publish"
                        Effect    = "Allow"
                        Principal = {
                            Service = "events.amazonaws.com"
                        }
                        Resource  = "arn:aws:sns:eu-central-1:553637109631:javabin-alerts"
                        Sid       = "AllowEventsPublish"
                    },
                  + {
                      + Action    = "SNS:Publish"
                      + Condition = {
                          + ArnLike      = {
                              + "aws:SourceArn" = "arn:aws:cloudwatch:eu-central-1:553637109631:alarm:*"
                            }
                          + StringEquals = {
                              + "aws:SourceAccount" = "553637109631"
                            }
                        }
                      + Effect    = "Allow"
                      + Principal = {
                          + Service = "cloudwatch.amazonaws.com"
                        }
                      + Resource  = "arn:aws:sns:eu-central-1:553637109631:javabin-alerts"
                      + Sid       = "AllowCloudWatchAlarmsPublish"
                    },
                ]
                # (1 unchanged attribute hidden)
            }
        )
        # (2 unchanged attributes hidden)
    }

Plan: 2 to add, 7 to change, 0 to destroy.

─────────────────────────────────────────────────────────────────────────────

Saved the plan to: tfplan

To perform exactly these actions, run the following command to apply:
    terraform apply "tfplan"

LLM Review

Risk: 🟡 MEDIUM

Plan adds 2 CloudWatch resources (metric filter and alarm) and updates 7 existing resources: Cognito user pool admin-only sign-up, Lambda code and env vars for auth/provisioning, an IAM policy widening DynamoDB and SSM read access, and an SNS topic policy extension; 0 destroyed.
[{"category":"security","detail":"Cognito user pool aws_cognito_user_pool.internal changes allow_admin_create_user_only from false to true. This disables self-signup, so only admins can create users. It is a tightening change, but it will affect any self-registration flow that depends on it. Confirm this is intended."},{"category":"security","detail":"IAM role policy javabin-app-auth-sync gains dynamodb:GetItem (the existing DeleteItem is unchanged) and three new SSM parameter ARNs: cognito-client-secret, cognito-client-identity-provider and auth-status under /javabin/apps//. The Lambda can now read client secrets for all apps. Scope is wildcarded across apps, so confirm the lambda needs read access to these secrets and that the wildcard is acceptable. The hidden statements are not shown as changed, so the Effect of the new actions is Allow in the existing statement."},{"category":"security","detail":"SNS topic policy javabin-alerts adds an AllowCloudWatchAlarmsPublish statement for cloudwatch.amazonaws.com, scoped by aws:SourceArn to alarms in this account and aws:SourceAccount. This is properly constrained and is needed for the new alarm."},{"category":"routine","detail":"Lambda functions auth_claims, app_auth_sync, app_secret_rollout and team_provisioner have source code hash changes (new deployment packages). auth_claims and app_auth_sync also add environment variables for federated identity providers (GoogleSAML, COGNITO), pool domains and REQUIRED_EMAIL_DOMAIN=java.no. EXTERNAL_POOL_DOMAIN is set to an empty string, so verify that the app_auth_sync code handles this. No destruction or replacement."},{"category":"routine","detail":"New CloudWatch metric filter and alarm (javabin-app-auth-sync-skipped) on the app-auth-sync log group, notifying the javabin-alerts SNS topic. Low cost (one alarm and one metric filter). Alarm threshold 0 with treat_missing_data=notBreaching is reasonable for a sparse error signal."}]

@Alexanderamiri

Copy link
Copy Markdown
Member Author

Review round 1 addressed (pushed 8359c32, rebased on #166). Notes on the new plan and risk review (MEDIUM, 2 add / 7 change / 0 destroy):

  • The plan includes fix(identity): lock down the internal Cognito pool #166's changes (allow_admin_create_user_only, auth-claims REQUIRED_EMAIL_DOMAIN / FEDERATED_IDENTITY_PROVIDERS, the skipped-sync metric filter and alarm, and the SNS policy for CloudWatch alarms), because this branch is stacked on it. Merge fix(identity): lock down the internal Cognito pool #166 first.
  • The metric filter pattern now also counts Sync failed for (a reconcile that could not sync an app, which no longer raises).
  • INTERNAL_IDENTITY_PROVIDERS = GoogleSAML matches cupcake's live client (SupportedIdentityProviders=[GoogleSAML]), so the post-apply reconcile changes no client and does not bring COGNITO back. The superseded comment above said GoogleSAML,COGNITO; that default is gone.
  • IAM: cognito-client-secret was already in WriteClientCredentials. The new ARNs are write grants for cognito-client-identity-provider and auth-status. Reading /javabin/apps/* comes from the unchanged ReadAuthRequests statement. The secret is never decrypted (WithDecryption=False).
  • EXTERNAL_POOL_DOMAIN = "" is handled: auth-status reports an empty pool_domain and the doctor skips its HTTP probes.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant