diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 310c6e74..edd88c6a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -557,6 +557,13 @@ jobs: - name: Terraform Format Check run: terraform fmt -check -recursive terraform/ + - name: Test Azure federation identity guards + if: matrix.cloud == 'azure' + run: | + terraform -chdir=iac/federation/azure-target/terraform init -backend=false -lockfile=readonly -input=false + terraform -chdir=iac/federation/azure-target/terraform fmt -check -recursive + terraform -chdir=iac/federation/azure-target/terraform test + - name: Terraform Init run: | cd terraform/environments/${{ matrix.cloud }} diff --git a/iac/federation/azure-target/terraform/.terraform.lock.hcl b/iac/federation/azure-target/terraform/.terraform.lock.hcl index fe2e1f97..68858923 100644 --- a/iac/federation/azure-target/terraform/.terraform.lock.hcl +++ b/iac/federation/azure-target/terraform/.terraform.lock.hcl @@ -3,9 +3,10 @@ provider "registry.terraform.io/hashicorp/azuread" { version = "3.8.0" - constraints = ">= 2.47.0" + constraints = "~> 3.8" hashes = [ "h1:E2YWNE3Qry4bQMlmmZ33X4hLY5hOGrEZrlRg4anI2uw=", + "h1:kIEmiknJFWKe44U9ePG5vXwoloKep0Dbi/mQ8uqCbw0=", "zh:0d26cfbf9417acd1c2295ccd5b0052abeac85ad1c3f6422ff09bf6a1ce16f00d", "zh:144d4ea92fed541a6376bc76ad65ba4738dfd7bcab4c9d6cc20d35001338d06d", "zh:1c3e89cf19118fc07d7b04257251fc9897e722c16e0a0df7b07fcd261f8c12e7", @@ -23,9 +24,10 @@ provider "registry.terraform.io/hashicorp/azuread" { provider "registry.terraform.io/hashicorp/azurerm" { version = "4.67.0" - constraints = ">= 3.95.0" + constraints = ">= 3.0.0, ~> 4.0" hashes = [ "h1:cT/1xU2vHvVboTZvjNjn7sqCH/FPP//JeMh702eGUS0=", + "h1:uBd8sVmk+t3yvxDP3tscD8VloIma0F79CRzNXQowOLY=", "zh:49a1531297510b684103c06d32e9ef1d810380ed55895f6b799913e9679141dc", "zh:4b0a8e5158296c1c02ab64e2374adf9df557fe1eb8191dffd158ed04a648bb3e", "zh:55ac4268c17bdb6f9bb9b6d761269e0e34fb88be14c7759dbcbaa9de9c94b23c", @@ -45,6 +47,7 @@ provider "registry.terraform.io/hashicorp/http" { version = "3.5.0" constraints = ">= 3.4.0" hashes = [ + "h1:8bUoPwS4hahOvzCBj6b04ObLVFXCEmEN8T/5eOHmWOM=", "h1:dl73+8wzQR++HFGoJgDqY3mj3pm14HUuH/CekVyOj5s=", "zh:047c5b4920751b13425efe0d011b3a23a3be97d02d9c0e3c60985521c9c456b7", "zh:157866f700470207561f6d032d344916b82268ecd0cf8174fb11c0674c8d0736", diff --git a/iac/federation/azure-target/terraform/tests/identity_guards.tftest.hcl b/iac/federation/azure-target/terraform/tests/identity_guards.tftest.hcl new file mode 100644 index 00000000..170a5387 --- /dev/null +++ b/iac/federation/azure-target/terraform/tests/identity_guards.tftest.hcl @@ -0,0 +1,233 @@ +mock_provider "azuread" {} +mock_provider "http" {} +mock_provider "azurerm" { + mock_data "azurerm_subscription" { + defaults = { + id = "/subscriptions/11111111-1111-1111-1111-111111111111" + subscription_id = "11111111-1111-1111-1111-111111111111" + tenant_id = "22222222-2222-2222-2222-222222222222" + } + } +} + +variables { + subscription_id = "11111111-1111-1111-1111-111111111111" + tenant_id = "22222222-2222-2222-2222-222222222222" + cudly_issuer_url = "https://cudly.example.com/oidc" + cudly_api_url = "" +} + +run "defaults" { + command = plan + assert { + condition = ( + azuread_application_federated_identity_credential.cudly.issuer == var.cudly_issuer_url && + azuread_application_federated_identity_credential.cudly.subject == "cudly-controller" && + azuread_application_federated_identity_credential.cudly.audiences == tolist(["api://AzureADTokenExchange"]) + ) + error_message = "Default identity must be preserved exactly." + } +} + +run "literal_claims" { + command = plan + variables { + cudly_issuer_url = "https://CUDly.example.com:8443/a_b/~v1%20/oidc" + cudly_federated_subject = "quote\"backslash\\backtick`apostrophe':é" + cudly_federated_audience = "quote\"backslash\\backtick`apostrophe':é" + } + assert { + condition = ( + azuread_application_federated_identity_credential.cudly.issuer == var.cudly_issuer_url && + azuread_application_federated_identity_credential.cudly.subject == var.cudly_federated_subject && + azuread_application_federated_identity_credential.cudly.audiences == tolist([var.cudly_federated_audience]) + ) + error_message = "Literal identity bytes and singleton audience must be preserved." + } +} + +run "invalid_claim_0" { + command = plan + variables { + cudly_federated_subject = null + cudly_federated_audience = null + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_1" { + command = plan + variables { + cudly_federated_subject = "" + cudly_federated_audience = "" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_2" { + command = plan + variables { + cudly_federated_subject = "a b" + cudly_federated_audience = "a b" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_3" { + command = plan + variables { + cudly_federated_subject = "a\tb" + cudly_federated_audience = "a\tb" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_4" { + command = plan + variables { + cudly_federated_subject = "a\nb" + cudly_federated_audience = "a\nb" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_5" { + command = plan + variables { + cudly_federated_subject = "a\rb" + cudly_federated_audience = "a\rb" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_6" { + command = plan + variables { + cudly_federated_subject = "a\u000bb" + cudly_federated_audience = "a\u000bb" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_7" { + command = plan + variables { + cudly_federated_subject = "a\u000cb" + cudly_federated_audience = "a\u000cb" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_8" { + command = plan + variables { + cudly_federated_subject = "a$b" + cudly_federated_audience = "a$b" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_9" { + command = plan + variables { + cudly_federated_subject = "a*b" + cudly_federated_audience = "a*b" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_issuer_0" { + command = plan + variables { + cudly_issuer_url = null + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_1" { + command = plan + variables { + cudly_issuer_url = "" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_2" { + command = plan + variables { + cudly_issuer_url = "http://example.com/oidc" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_3" { + command = plan + variables { + cudly_issuer_url = "https://" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_4" { + command = plan + variables { + cudly_issuer_url = "https://example.com/oidc/" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_5" { + command = plan + variables { + cudly_issuer_url = "https://user@example.com/oidc" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_6" { + command = plan + variables { + cudly_issuer_url = "https://example.com/?q=1" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_7" { + command = plan + variables { + cudly_issuer_url = "https://example.com/#f" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_8" { + command = plan + variables { + cudly_issuer_url = "https://example.com/a b" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_9" { + command = plan + variables { + cudly_issuer_url = "https://example.com/a\nb" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_10" { + command = plan + variables { + cudly_issuer_url = "https://example.com/a\"b" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_11" { + command = plan + variables { + cudly_issuer_url = "https://example.com/a\\b" + } + expect_failures = [var.cudly_issuer_url] +} diff --git a/iac/federation/azure-target/terraform/variables.tf b/iac/federation/azure-target/terraform/variables.tf index aced3d38..50e25f0a 100644 --- a/iac/federation/azure-target/terraform/variables.tf +++ b/iac/federation/azure-target/terraform/variables.tf @@ -19,18 +19,37 @@ variable "app_display_name" { variable "cudly_issuer_url" { description = "CUDly OIDC issuer URL (e.g. https://cudly.example.com/oidc). Azure AD fetches JWKS from this issuer to verify client assertion JWTs." type = string + + # CUDly contract, mirrored by the two azure-wif shell templates. + validation { + condition = var.cudly_issuer_url == null ? false : ( + can(regex("^https://[A-Za-z0-9.-]+(:[0-9]+)?(/[A-Za-z0-9._~%/-]*)?$", var.cudly_issuer_url)) && + !endswith(var.cudly_issuer_url, "/") + ) + error_message = "Use a CUDly HTTPS issuer URL without a trailing slash, query, fragment or userinfo." + } } variable "cudly_federated_subject" { description = "Subject claim in the client assertion JWT. Must match what CUDly signs." type = string default = "cudly-controller" + + validation { + condition = var.cudly_federated_subject == null ? false : can(regex("^[^ \\t\\n\\r\\x0b\\f$*]+$", var.cudly_federated_subject)) + error_message = "Subject must be nonempty without ASCII whitespace, dollar signs or asterisks." + } } variable "cudly_federated_audience" { description = "Audience for the federated identity credential." type = string default = "api://AzureADTokenExchange" + + validation { + condition = var.cudly_federated_audience == null ? false : can(regex("^[^ \\t\\n\\r\\x0b\\f$*]+$", var.cudly_federated_audience)) + error_message = "Audience must be nonempty without ASCII whitespace, dollar signs or asterisks." + } } variable "cudly_api_url" { diff --git a/internal/iacfiles/templates/README.md b/internal/iacfiles/templates/README.md index 42b09adf..2042e433 100644 --- a/internal/iacfiles/templates/README.md +++ b/internal/iacfiles/templates/README.md @@ -52,6 +52,11 @@ from this directory via the `//go:embed` directive in `internal/iacfiles/embed.g Use `scripts/generate-federation-iac.go`, a self-contained Go script with no external dependencies. +Azure targets require `--cudly-api-url` with the CUDly HTTPS base URL, without a +trailing slash. The generator appends `/oidc` for the issuer and preserves custom +paths. Contact email remains optional; AWS and GCP targets may omit the API URL +to skip automatic registration. + Every AWS target with a non-AWS source requires `--oidc-subject-claim`: it is the workload subject the generated AWS trust policy pins to, and there is no working default (see #1640). Pass the calling workload's subject claim, which is @@ -101,7 +106,8 @@ go run scripts/generate-federation-iac.go \ go run scripts/generate-federation-iac.go \ --target azure --source aws \ --account-name "prod-azure" --account-id "sub-xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" \ - --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" \ + --cudly-api-url "https://cudly.example.com" # GCP target, AWS source go run scripts/generate-federation-iac.go \ diff --git a/internal/iacfiles/templates/azure-wif-cli.sh.tmpl b/internal/iacfiles/templates/azure-wif-cli.sh.tmpl index fafaf495..6bfc7a97 100644 --- a/internal/iacfiles/templates/azure-wif-cli.sh.tmpl +++ b/internal/iacfiles/templates/azure-wif-cli.sh.tmpl @@ -5,7 +5,7 @@ # Usage: # bash {{.AccountSlug}}-azure-wif-cli.sh # -# Requirements: az cli logged in to the target subscription. +# Requirements: jq and az cli logged in to the target subscription. # # This script configures true Azure Workload Identity Federation — # no certificate, no private key, no client secret is created or stored. @@ -22,11 +22,24 @@ SUBSCRIPTION_ID="${SUBSCRIPTION_ID:-{{.SubscriptionID}}}" # /.well-known/openid-configuration when resolving the discovery doc, # so do NOT include the well-known suffix here. Trailing slashes and # http/https matter for Azure AD validation (AADSTS700213 if mismatched). -CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL:-{{.CUDlyAPIURL}}/oidc}" -CUDLY_FEDERATED_SUBJECT="${CUDLY_FEDERATED_SUBJECT:-cudly-controller}" -CUDLY_FEDERATED_AUDIENCE="${CUDLY_FEDERATED_AUDIENCE:-api://AzureADTokenExchange}" +CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL-{{.CUDlyAPIURL}}/oidc}" +CUDLY_FEDERATED_SUBJECT="${CUDLY_FEDERATED_SUBJECT-cudly-controller}" +CUDLY_FEDERATED_AUDIENCE="${CUDLY_FEDERATED_AUDIENCE-api://AzureADTokenExchange}" -: "${CUDLY_ISSUER_URL:?set CUDLY_ISSUER_URL to the OIDC issuer URL of your CUDly deployment (base URL + /oidc)}" +# Keep this CUDly input contract aligned with azure-target/terraform/variables.tf. +ISSUER_PATTERN='^https://[A-Za-z0-9.-]+(:[0-9]+)?(/[A-Za-z0-9._~%/-]*)?$' +if [[ ! "${CUDLY_ISSUER_URL}" =~ $ISSUER_PATTERN || "${CUDLY_ISSUER_URL}" == */ ]]; then + echo "ERROR: CUDLY_ISSUER_URL must be a CUDly HTTPS issuer URL without a trailing slash." >&2 + exit 1 +fi +for CLAIM_NAME in CUDLY_FEDERATED_SUBJECT CUDLY_FEDERATED_AUDIENCE; do + case "${!CLAIM_NAME}" in + ''|*[$' \t\n\r\v\f']*|*'$'*|*'*'*) + echo "ERROR: ${CLAIM_NAME} must be nonempty without ASCII whitespace, dollar signs or asterisks." >&2 + exit 1 ;; + esac +done +command -v jq >/dev/null 2>&1 || { echo "ERROR: jq is required." >&2; exit 1; } if [[ -n "${SUBSCRIPTION_ID}" && "${SUBSCRIPTION_ID}" != "" ]]; then az account set --subscription "${SUBSCRIPTION_ID}" @@ -41,23 +54,16 @@ echo "Creating service principal..." SP_OBJECT_ID=$(az ad sp create --id "${APP_ID}" --query id --output tsv) echo "Adding federated identity credential bound to CUDly OIDC issuer..." -# Idempotent: if the same name exists (re-run of the script), az throws a -# conflict error — catch and continue. A subsequent run with the same -# script is safe. -FEDCRED_PARAMS=$(cat <&1 || echo " (federated credential may already exist — continuing)" + --output none echo "" echo "=== Done ===" diff --git a/internal/iacfiles/templates/azure-wif-deploy.sh.tmpl b/internal/iacfiles/templates/azure-wif-deploy.sh.tmpl index 5f6bfae8..3966d606 100644 --- a/internal/iacfiles/templates/azure-wif-deploy.sh.tmpl +++ b/internal/iacfiles/templates/azure-wif-deploy.sh.tmpl @@ -10,7 +10,7 @@ # # Set CUDLY_CONTACT_EMAIL to auto-register with CUDly after deployment. # -# Requirements: az cli v2 logged in to the target subscription. +# Requirements: jq and az cli v2 logged in to the target subscription. set -euo pipefail APP_NAME="${APP_NAME:-CUDly-target}" @@ -25,6 +25,25 @@ while [[ $# -gt 0 ]]; do esac done +CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL-{{.CUDlyAPIURL}}/oidc}" +CUDLY_FEDERATED_SUBJECT="${CUDLY_FEDERATED_SUBJECT-cudly-controller}" +CUDLY_FEDERATED_AUDIENCE="${CUDLY_FEDERATED_AUDIENCE-api://AzureADTokenExchange}" + +# Keep this CUDly input contract aligned with azure-target/terraform/variables.tf. +ISSUER_PATTERN='^https://[A-Za-z0-9.-]+(:[0-9]+)?(/[A-Za-z0-9._~%/-]*)?$' +if [[ ! "${CUDLY_ISSUER_URL}" =~ $ISSUER_PATTERN || "${CUDLY_ISSUER_URL}" == */ ]]; then + echo "ERROR: CUDLY_ISSUER_URL must be a CUDly HTTPS issuer URL without a trailing slash." >&2 + exit 1 +fi +for CLAIM_NAME in CUDLY_FEDERATED_SUBJECT CUDLY_FEDERATED_AUDIENCE; do + case "${!CLAIM_NAME}" in + ''|*[$' \t\n\r\v\f']*|*'$'*|*'*'*) + echo "ERROR: ${CLAIM_NAME} must be nonempty without ASCII whitespace, dollar signs or asterisks." >&2 + exit 1 ;; + esac +done +command -v jq >/dev/null 2>&1 || { echo "ERROR: jq is required." >&2; exit 1; } + DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" case "$TEMPLATE" in @@ -57,26 +76,18 @@ echo "SP Object ID: ${SP_OBJECT_ID}" # --------------------------------------------------------------------------- # Step 3: Add federated identity credential bound to CUDly OIDC issuer # --------------------------------------------------------------------------- -CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL:-{{.CUDlyAPIURL}}/oidc}" -CUDLY_FEDERATED_SUBJECT="${CUDLY_FEDERATED_SUBJECT:-cudly-controller}" -CUDLY_FEDERATED_AUDIENCE="${CUDLY_FEDERATED_AUDIENCE:-api://AzureADTokenExchange}" - echo "" echo "Adding federated identity credential..." -FEDCRED_PARAMS=$(cat <&1 || echo " (federated credential may already exist — continuing)" + --output none # --------------------------------------------------------------------------- # Step 4: Deploy role assignment via Bicep/ARM diff --git a/internal/iacfiles/templates_azure_wif_test.go b/internal/iacfiles/templates_azure_wif_test.go new file mode 100644 index 00000000..0fcee1f8 --- /dev/null +++ b/internal/iacfiles/templates_azure_wif_test.go @@ -0,0 +1,144 @@ +package iacfiles + +import ( + "encoding/json" + "fmt" + "regexp" + "strings" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/iac" + "github.com/stretchr/testify/require" +) + +func TestAzureWIFTerraformPlansOnly(t *testing.T) { + data, err := iac.Modules.ReadFile("federation/azure-target/terraform/tests/identity_guards.tftest.hcl") + require.NoError(t, err) + runs := regexp.MustCompile(`(?m)^run "[^"]+" \{`).FindAll(data, -1) + plans := regexp.MustCompile(`(?m)^run "[^"]+" \{\s*command\s*=\s*plan\s`).FindAll(data, -1) + require.NotEmpty(t, runs) + require.Len(t, plans, len(runs), "every mock run must explicitly start with command = plan") + commands := regexp.MustCompile(`(?m)^\s*command\s*=\s*(\w+)`).FindAllSubmatch(data, -1) + for _, command := range commands { + require.Equal(t, "plan", string(command[1]), "apply is forbidden in this mock suite") + } +} + +func runAzureWIF(t *testing.T, template string, data testTemplateData, env map[string]string) (int, string, string, []string) { + t.Helper() + return runRenderedScript(t, "azure-wif.sh", renderCLITemplate(t, template, data), func(logPath string) map[string]string { + return map[string]string{ + "az": fmt.Sprintf(`#!/usr/bin/env bash +set -euo pipefail +printf 'az %%s\n' "$*" >> %q +case "$1 $2" in + 'account set') ;; + 'account show') echo 11111111-1111-1111-1111-111111111111 ;; + 'ad app') + if [[ "$3" == 'federated-credential' ]]; then + [[ "${FAIL_CREDENTIAL:-}" == '' ]] || { echo "$FAIL_CREDENTIAL" >&2; exit 42; } + while [[ $# -gt 0 ]]; do + if [[ "$1" == '--parameters' ]]; then + jq -c . <<< "$2" >> %q + exit + fi + shift + done + exit 43 + fi + echo 22222222-2222-2222-2222-222222222222 ;; + 'ad sp') echo 33333333-3333-3333-3333-333333333333 ;; + 'deployment sub') ;; + *) exit 44 ;; +esac +`, logPath, logPath), + "curl": fmt.Sprintf("#!/usr/bin/env bash\necho curl >> %q\necho 409\n", logPath), + } + }, env) +} + +func TestAzureWIFInputGuards(t *testing.T) { + for _, template := range []string{"templates/azure-wif-cli.sh.tmpl", "templates/azure-wif-deploy.sh.tmpl"} { + t.Run(template, func(t *testing.T) { + for _, key := range []string{"CUDLY_FEDERATED_SUBJECT", "CUDLY_FEDERATED_AUDIENCE"} { + for _, value := range []string{"", "a b", "a\tb", "a\nb", "a\rb", "a\vb", "a\fb", "a$b", "a*b"} { + t.Run(key+fmt.Sprintf("%q", value), func(t *testing.T) { + code, _, stderr, calls := runAzureWIF(t, template, baseData(), map[string]string{key: value}) + require.NotZero(t, code) + require.Empty(t, calls) + require.Contains(t, stderr, key) + }) + } + } + for _, value := range []string{"", "http://example.com/oidc", "https://", "https://example.com/oidc/", "https://user@example.com/oidc", "https://example.com/?q=1", "https://example.com/#f", "https://example.com/a b", "https://example.com/a\nb", `https://example.com/a"b`, `https://example.com/a\b`} { + t.Run(fmt.Sprintf("issuer%q", value), func(t *testing.T) { + code, _, stderr, calls := runAzureWIF(t, template, baseData(), map[string]string{"CUDLY_ISSUER_URL": value}) + require.NotZero(t, code) + require.Empty(t, calls) + require.Contains(t, stderr, "CUDLY_ISSUER_URL") + }) + } + t.Run("missing rendered issuer", func(t *testing.T) { + data := baseData() + data.CUDlyAPIURL = "" + code, _, _, calls := runAzureWIF(t, template, data, nil) + require.NotZero(t, code) + require.Empty(t, calls) + }) + t.Run("missing jq", func(t *testing.T) { + code, _, stderr, calls := runAzureWIF(t, template, baseData(), map[string]string{"PATH": t.TempDir()}) + require.NotZero(t, code) + require.Empty(t, calls) + require.Contains(t, stderr, "jq is required") + }) + }) + } +} + +func TestAzureWIFFederatedCredential(t *testing.T) { + for _, template := range []string{"templates/azure-wif-cli.sh.tmpl", "templates/azure-wif-deploy.sh.tmpl"} { + t.Run(template, func(t *testing.T) { + for _, failure := range []string{"Authorization_RequestDenied", "TooManyRequests", "Conflict"} { + t.Run(failure, func(t *testing.T) { + code, stdout, stderr, calls := runAzureWIF(t, template, baseData(), map[string]string{"FAIL_CREDENTIAL": failure}) + require.Equal(t, 42, code) + require.Contains(t, stdout+stderr, failure) + require.NotContains(t, stdout, "=== Done ===") + require.NotContains(t, strings.Join(calls, "\n"), "az deployment") + require.NotContains(t, calls, "curl") + }) + } + for _, literal := range []string{"", "quote\"backslash\\backtick`apostrophe':é"} { + t.Run("literal/"+literal, func(t *testing.T) { + issuer, subject, audience := baseData().CUDlyAPIURL+"/oidc", "cudly-controller", "api://AzureADTokenExchange" + env := map[string]string{} + if literal != "" { + issuer, subject, audience = "https://CUDly.example.com:8443/a_b/~v1%20/oidc", literal, literal + env = map[string]string{"CUDLY_ISSUER_URL": issuer, "CUDLY_FEDERATED_SUBJECT": subject, "CUDLY_FEDERATED_AUDIENCE": audience} + } + code, stdout, stderr, calls := runAzureWIF(t, template, baseData(), env) + require.Zero(t, code, "%s", stderr) + require.Contains(t, stdout, "=== Done ===") + require.Contains(t, calls, "curl") + if strings.Contains(template, "deploy") { + require.Contains(t, strings.Join(calls, "\n"), "az deployment sub create") + } + var credentials []map[string]any + for _, call := range calls { + if strings.HasPrefix(call, "{") { + var credential map[string]any + require.NoError(t, json.Unmarshal([]byte(call), &credential)) + credentials = append(credentials, credential) + } + } + require.Len(t, credentials, 1) + require.Len(t, credentials[0], 5) + require.Equal(t, "cudly", credentials[0]["name"]) + require.Equal(t, issuer, credentials[0]["issuer"]) + require.Equal(t, subject, credentials[0]["subject"]) + require.Equal(t, []any{audience}, credentials[0]["audiences"]) + }) + } + }) + } +} diff --git a/internal/iacfiles/templates_test.go b/internal/iacfiles/templates_test.go index 58c55158..e7bc2d64 100644 --- a/internal/iacfiles/templates_test.go +++ b/internal/iacfiles/templates_test.go @@ -126,13 +126,13 @@ func TestCLITemplatesAutoRegister(t *testing.T) { `ACCOUNT_NAME="${CUDLY_ACCOUNT_NAME:-Azure ${SUBSCRIPTION_ID}}"`, // Secret-free redesign: must use federated identity credential, not a cert upload. "az ad app federated-credential create", - `"issuer": "${CUDLY_ISSUER_URL}"`, - `"subject": "${CUDLY_FEDERATED_SUBJECT}"`, - `"audiences": ["${CUDLY_FEDERATED_AUDIENCE}"]`, + `issuer: $issuer`, + `subject: $subject`, + `audiences: [$audience]`, // Issuer env var must default to the CUDly base URL + /oidc // so Azure AD appending /.well-known/openid-configuration // resolves to the discovery endpoint on the CUDly deployment. - `CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL:-https://cudly.example.com/oidc}"`, + `CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL-https://cudly.example.com/oidc}"`, }, mustNot: []string{ "/api/registrations", diff --git a/scripts/generate-federation-iac.go b/scripts/generate-federation-iac.go index 10de1b57..a671601c 100644 --- a/scripts/generate-federation-iac.go +++ b/scripts/generate-federation-iac.go @@ -62,7 +62,8 @@ // go run scripts/generate-federation-iac.go \ // --target azure --source aws \ // --account-name "prod-azure" --account-id "sub-xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" \ -// --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" +// --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" \ +// --cudly-api-url "https://cudly.example.com" // // # GCP target, AWS source — WIF pool tfvars // # --source-account-id is CUDly's own AWS account (required when --source aws) @@ -131,13 +132,8 @@ type iacData struct { ProjectID string ServiceAccountEmail string OIDCIssuerURI string - // CUDlyAPIURL and ContactEmail feed the optional auto-registration block - // in the tfvars/deploy-script templates. The server pre-fills them from - // the dashboard URL and the authenticated session; the standalone script - // has neither, so they come from --cudly-api-url / --contact-email and - // default to "" (which the Terraform modules treat as "skip - // registration" — see the cudly_api_url/contact_email variable - // descriptions in iac/federation/*/terraform/variables.tf). + // Azure also requires CUDlyAPIURL to derive its issuer; ContactEmail is optional. + // Other targets use both fields only for optional registration. CUDlyAPIURL string ContactEmail string } @@ -504,13 +500,20 @@ func requireSourceAccountID(data *iacData, source, sourceAccountID, errMsg strin return nil } -// populateData fills target-specific fields on data from CLI flags. It reports -// an error rather than writing an invalid value into data, so a rejected flag -// stops the run before any template is rendered. This covers two independent -// gates: --oidc-subject-claim, required (and only meaningful) for an AWS -// target with a non-AWS source; and --source-account-id, required whenever -// the source is AWS itself (the aws-cross-account and gcp-wif-from-aws paths), -// since CUDly's own account has no other way to reach this standalone script. +// Mirrors iac/federation/azure-target/terraform/variables.tf's issuer grammar. +var azureAPIURLRE = regexp.MustCompile(`^https://[A-Za-z0-9.-]+(:\d+)?(/[A-Za-z0-9._~%/-]*)?$`) + +func validateAzureAPIURL(baseURL string) error { + if baseURL == "" { + return errors.New("--cudly-api-url is required when --target=azure to derive the CUDly OIDC issuer") + } + if !azureAPIURLRE.MatchString(baseURL) || strings.HasSuffix(baseURL, "/") { + return errors.New("--cudly-api-url must be a CUDly HTTPS base URL without a trailing slash, query, fragment or userinfo") + } + return nil +} + +// Validation precedes rendering so rejected input cannot overwrite an artifact. func populateData(data *iacData, target, source, tenantID, projectID, saEmail, oidcSubjectClaim, sourceAccountID string) error { // --target is checked first so that a typo there is reported as a bad // --target rather than as an inapplicable --oidc-subject-claim, which would @@ -536,6 +539,7 @@ func populateData(data *iacData, target, source, tenantID, projectID, saEmail, o case "azure": data.SubscriptionID = data.AccountExternalID data.TenantID = tenantID + return validateAzureAPIURL(data.CUDlyAPIURL) case "gcp": data.ProjectID = projectID if data.ProjectID == "" { @@ -573,7 +577,7 @@ func main() { oidcSubjectClaim := flag.String("oidc-subject-claim", "", "Subject (sub) claim restricting the AWS trust policy to one workload: a GCP service account's numeric unique ID or an Azure managed identity's object ID. Required when --target=aws and --source is not aws; there is no working default (see #1640). Letters, digits and . _ : / @ = + - only") sourceAccountID := flag.String("source-account-id", "", "AWS account ID where CUDly itself runs (required for --target aws --source aws, and --target gcp --source aws; NOT the same as --account-id, which is the target account)") contactEmail := flag.String("contact-email", "", "Contact email pre-filled for CUDly auto-registration (optional; empty skips auto-registration)") - cudlyAPIURL := flag.String("cudly-api-url", "", "CUDly API base URL pre-filled for auto-registration (optional; empty skips auto-registration)") + cudlyAPIURL := flag.String("cudly-api-url", "", "CUDly HTTPS base URL without a trailing slash (required for Azure: /oidc is appended for its issuer; otherwise optional for auto-registration)") outFile := flag.String("output", "", "Output file path; use '-' to print to stdout (default: derived filename in current directory)") templDir := flag.String("templates-dir", "internal/iacfiles/templates", "Path to templates directory (run from repo root)") modulesDir := flag.String("modules-dir", "iac/federation", "Path to Terraform modules directory (used by --format bundle)") diff --git a/scripts/generate-federation-iac_test.go b/scripts/generate-federation-iac_test.go index 9e468be7..0b3a29f4 100644 --- a/scripts/generate-federation-iac_test.go +++ b/scripts/generate-federation-iac_test.go @@ -11,9 +11,15 @@ package main // nothing exercised this path so the break went unnoticed. import ( + "archive/zip" + "io" + "os" "os/exec" + "path/filepath" "strings" "testing" + + "github.com/stretchr/testify/require" ) // runViaGoRun invokes `go run generate-federation-iac.go ` from the @@ -263,3 +269,128 @@ func TestGenerateFederationIaC_RejectsMalformedSourceAccountID(t *testing.T) { }) } } + +func TestGenerator_AzureRejectsInvalidAPIURL(t *testing.T) { + cases := []struct { + name string + url string + omit bool + }{ + {name: "omitted", omit: true}, + {name: "empty"}, + {name: "http", url: "http://cudly.example.com"}, + {name: "missing host", url: "https://"}, + {name: "space", url: "https://cudly.example.com/a b"}, + {name: "newline", url: "https://cudly.example.com/a\nb"}, + {name: "quote", url: `https://cudly.example.com/a"b`}, + {name: "backslash", url: `https://cudly.example.com/a\b`}, + {name: "userinfo", url: "https://user@cudly.example.com"}, + {name: "query", url: "https://cudly.example.com/?q=1"}, + {name: "fragment", url: "https://cudly.example.com/#f"}, + {name: "trailing slash", url: "https://cudly.example.com/"}, + {name: "nonnumeric port", url: "https://cudly.example.com:https"}, + } + for _, source := range []string{"aws", "gcp", "azure"} { + for _, format := range []string{"tfvars", "bundle"} { + for _, tc := range cases { + t.Run(source+"/"+format+"/"+tc.name, func(t *testing.T) { + output := filepath.Join(t.TempDir(), "output") + args := []string{ + "--target", "azure", "--source", source, + "--account-name", "Acme", "--account-id", "11111111-2222-3333-4444-555555555555", + "--tenant-id", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", "--output", output, + } + if format == "bundle" { + args = append(args, "--format", "bundle") + } + if !tc.omit { + args = append(args, "--cudly-api-url", tc.url) + } + res := runGenerator(t, args...) + require.NotZero(t, res.exitCode, "stdout: %s; stderr: %s", res.stdout, res.stderr) + require.Contains(t, res.stderr, "--cudly-api-url") + require.Empty(t, res.stdout) + _, err := os.Stat(output) + require.ErrorIs(t, err, os.ErrNotExist) + }) + } + } + } +} + +func TestGenerator_AzureMissingURLPreservesOutput(t *testing.T) { + for _, format := range []string{"tfvars", "bundle"} { + t.Run(format, func(t *testing.T) { + output := filepath.Join(t.TempDir(), "existing-output") + const sentinel = "existing operator data\n" + require.NoError(t, os.WriteFile(output, []byte(sentinel), 0o600)) + args := []string{"--target", "azure", "--source", "aws", "--account-name", "Acme", "--account-id", "subscription"} + if format == "bundle" { + args = append(args, "--format", "bundle") + } + res := runGenerator(t, append(args, "--output", output)...) + require.NotZero(t, res.exitCode, "stderr: %s", res.stderr) + require.Contains(t, res.stderr, "--cudly-api-url") + got, err := os.ReadFile(output) + require.NoError(t, err) + require.Equal(t, sentinel, string(got)) + if format == "tfvars" { + res = runGenerator(t, append(args, "--output", "-")...) + require.NotZero(t, res.exitCode) + require.Empty(t, res.stdout) + require.Contains(t, res.stderr, "--cudly-api-url") + } + }) + } +} + +func TestGenerator_AzurePreservesValidAPIURL(t *testing.T) { + for _, baseURL := range []string{"https://cudly.example.com", "https://CUDly.example.com:8443/a_b/~v1%20"} { + for _, source := range []string{"aws", "gcp", "azure"} { + for _, format := range []string{"tfvars", "bundle"} { + t.Run(baseURL+"/"+source+"/"+format, func(t *testing.T) { + args := []string{ + "--target", "azure", "--source", source, + "--account-name", "Acme", "--account-id", "11111111-2222-3333-4444-555555555555", + "--tenant-id", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", "--cudly-api-url", baseURL, + } + output := "-" + if format == "bundle" { + output = filepath.Join(t.TempDir(), "bundle.zip") + args = append(args, "--format", "bundle") + } + res := runGenerator(t, append(args, "--output", output)...) + require.Zero(t, res.exitCode, "stderr: %s", res.stderr) + content := res.stdout + if format == "bundle" { + archive, err := zip.OpenReader(output) + require.NoError(t, err) + defer archive.Close() + entry, err := archive.Open("terraform/acme-azure-wif.tfvars") + require.NoError(t, err) + data, err := io.ReadAll(entry) + require.NoError(t, entry.Close()) + require.NoError(t, err) + content = string(data) + } + require.Contains(t, content, `cudly_issuer_url = "`+baseURL+`/oidc"`) + require.Contains(t, content, `cudly_api_url = "`+baseURL+`"`) + require.Contains(t, content, `contact_email = ""`) + }) + } + } + } +} + +func TestGenerator_NonAzureDoesNotRequireAPIURL(t *testing.T) { + for _, target := range []string{"aws", "gcp"} { + t.Run(target, func(t *testing.T) { + res := runGenerator(t, + "--target", target, "--source", "aws", "--account-name", "Acme", "--account-id", "999888777666", + "--source-account-id", "111122223333", "--output", "-", + ) + require.Zero(t, res.exitCode, "stderr: %s", res.stderr) + require.Contains(t, res.stdout, `cudly_api_url = ""`) + }) + } +}