From e4f9ac9eb2e33884c9cf9c478f5547463fd46994 Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Thu, 27 Aug 2026 17:26:30 -0700 Subject: [PATCH 1/7] test(e2e): drive the oidc-credential chain from `formae connect` The oidc-credential e2e proved the exchange but not how the trust it exchanges against came to exist. The IAM identity provider and the role were hand-provisioned standing resources and the test asserted on a role name it could only have been told, so what `formae connect` actually provisions was never exercised end to end. The role is now made by the run that uses it. A stub control plane serves the cloud-connection setup and receives the registration, a stub auth plugin stands in for the hosted one the gate demands, and `formae connect aws --profile-aws` provisions the identity provider and the role for real before the agent starts. The echo plugin then assumes that role, so a trust policy connect writes wrongly fails the test instead of passing beside a correct one somebody made by hand. Three things the fixtures had pinned now travel from the run: - the role ARN, because it does not exist until connect makes it; - the subject, which moves to the grammar the control plane really produces, `fai:/`, rather than an invented shape production never emits and the GCP path cannot even parse; - the audience, since each cloud spells it differently. The exchange retries while IAM propagates. A role created seconds earlier is not immediately assumable and STS answers AccessDenied until it is, which failed the first real run against a trust policy that was correct. The window sits inside the agent's plugin call deadline, so a genuinely wrong policy still fails, at the end of it rather than the start. The standing forma keeps the issuer, its published keys and the identity provider, and drops the role. The provider stays because it is account-global and shared, and because connect validates and reuses an existing one rather than needing to create it. The per-run role carries the suite's own name prefix so the pre-cleanup purge reclaims one a dead run left behind. --- .github/workflows/e2e-tests.yml | 12 +- Makefile | 23 +- tests/e2e/config/nuke-config.yaml | 16 +- .../config/oidc-standing/oidc_standing.pkl | 49 +-- tests/e2e/go/connect.go | 321 +++++++++++++++++ .../fixtures/oidc-auth-stub/formae-plugin.pkl | 16 + tests/e2e/go/fixtures/oidc-auth-stub/go.mod | 7 + tests/e2e/go/fixtures/oidc-auth-stub/main.go | 51 +++ .../go/fixtures/oidc-credential-stub/main.go | 21 +- .../go/fixtures/oidc-echo-plugin/plugin.go | 329 +++++++++++++++--- tests/e2e/go/oidc_credential_test.go | 160 +++++++-- 11 files changed, 880 insertions(+), 125 deletions(-) create mode 100644 tests/e2e/go/connect.go create mode 100644 tests/e2e/go/fixtures/oidc-auth-stub/formae-plugin.pkl create mode 100644 tests/e2e/go/fixtures/oidc-auth-stub/go.mod create mode 100644 tests/e2e/go/fixtures/oidc-auth-stub/main.go diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index cb4f50307..dc77788da 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -177,11 +177,13 @@ jobs: - test: TestPluginConfig system_plugins: sftp # Installs no cloud plugin: the oidc-credential fixtures (a stub - # broker and a resource plugin that exchanges the token it is - # handed) are built from this repo and staged by `make test-e2e` - # itself. It does need AWS credentials — the broker reads its - # signing key from Secrets Manager and the plugin calls - # sts:AssumeRoleWithWebIdentity against the standing issuer. + # broker, a resource plugin that exchanges the token it is handed, + # and a stub hosted auth plugin) are built from this repo and staged + # by `make test-e2e` itself. It does need AWS credentials, and the + # `e2e-test` shared-config profile written above: `formae connect` + # provisions the role with it, the broker reads its signing key from + # Secrets Manager, and the plugin calls sts:AssumeRoleWithWebIdentity + # against the standing issuer. - test: TestOidcCredential steps: diff --git a/Makefile b/Makefile index 00449f427..344013a6f 100644 --- a/Makefile +++ b/Makefile @@ -267,20 +267,23 @@ test-e2e: build stage-oidc-fixtures E2E_FORMAE_BINARY=$(CURDIR)/dist/e2e/bin/formae \ E2E_OIDC_PLUGIN_DIR=$(OIDC_STAGE_DIR) \ E2E_OIDC_PLUGIN_DIR_NO_BROKER=$(OIDC_STAGE_DIR_NO_BROKER) \ + E2E_OIDC_AUTH_PLUGIN_DIR=$(OIDC_STAGE_DIR_AUTH) \ go test -C ./tests/e2e/go -tags=e2e -timeout 30m -v ./... $(E2E_RUN_FLAGS) -# The hermetic oidc-credential fixtures: a stub credential broker and a resource -# plugin that echoes the token the broker mints. Staged into two plugin trees -# the e2e agent can be pointed at — one where the broker sits beside the echo -# plugin, one where the echo plugin is alone — in the layout plugin discovery -# expects (//v/ beside the manifest). +# The oidc-credential fixtures: a stub credential broker, a resource plugin that +# echoes the token the broker mints, and a stub hosted auth plugin. Staged into +# three plugin trees — one where the broker sits beside the echo plugin, one +# where the echo plugin is alone, and one holding only the auth plugin, which the +# CLI reads rather than the agent — in the layout plugin discovery expects +# (//v/ beside the manifest). OIDC_FIXTURE_DIR := $(CURDIR)/tests/e2e/go/fixtures OIDC_STAGE_DIR := $(CURDIR)/dist/e2e/oidc-plugins OIDC_STAGE_DIR_NO_BROKER := $(CURDIR)/dist/e2e/oidc-plugins-no-broker +OIDC_STAGE_DIR_AUTH := $(CURDIR)/dist/e2e/oidc-auth-plugin stage-oidc-fixtures: @echo "Staging e2e oidc-credential fixtures..." - rm -rf $(OIDC_STAGE_DIR) $(OIDC_STAGE_DIR_NO_BROKER) + rm -rf $(OIDC_STAGE_DIR) $(OIDC_STAGE_DIR_NO_BROKER) $(OIDC_STAGE_DIR_AUTH) mkdir -p $(OIDC_STAGE_DIR)/oidc-credential-stub/v0.0.1 go build -C $(OIDC_FIXTURE_DIR)/oidc-credential-stub \ -o $(OIDC_STAGE_DIR)/oidc-credential-stub/v0.0.1/oidc-credential-stub . @@ -295,6 +298,14 @@ stage-oidc-fixtures: $(OIDC_STAGE_DIR)/oidc-echo/v0.0.1/schema/pkl mkdir -p $(OIDC_STAGE_DIR_NO_BROKER) cp -R $(OIDC_STAGE_DIR)/oidc-echo $(OIDC_STAGE_DIR_NO_BROKER)/oidc-echo + # Installed under the name `oidc` rather than the fixture directory's name: + # the CLI resolves an auth plugin by the `type` its profile's auth block + # names, and the connect gate admits only that one. + mkdir -p $(OIDC_STAGE_DIR_AUTH)/oidc/v0.0.1 + go build -C $(OIDC_FIXTURE_DIR)/oidc-auth-stub \ + -o $(OIDC_STAGE_DIR_AUTH)/oidc/v0.0.1/oidc . + cp $(OIDC_FIXTURE_DIR)/oidc-auth-stub/formae-plugin.pkl \ + $(OIDC_STAGE_DIR_AUTH)/oidc/v0.0.1/formae-plugin.pkl ## test-property: Run property tests (FullChaos 100 iterations, others 50) test-property: diff --git a/tests/e2e/config/nuke-config.yaml b/tests/e2e/config/nuke-config.yaml index b51d9ac76..07bcb96e5 100644 --- a/tests/e2e/config/nuke-config.yaml +++ b/tests/e2e/config/nuke-config.yaml @@ -154,12 +154,16 @@ presets: # resource types are protected by default (allowlist semantics). # # The same allowlist is what keeps the standing oidc-credential fixtures - # alive: the static issuer bucket, its IAM OIDC provider, the role the - # e2e assumes, and the Secrets Manager secret holding the signing key are - # all named e2e-oidc-*, contain no "formae-e2e", and are therefore never - # removed. They are provisioned once, out of band, from the forma in - # tests/e2e/config/oidc-standing/. Do not turn this preset into a - # denylist without re-homing them first. + # alive: the static issuer bucket, its IAM OIDC provider, and the Secrets + # Manager secret holding the signing key are all named e2e-oidc-*, contain + # no "formae-e2e", and are therefore never removed. They are provisioned + # once, out of band, from the forma in tests/e2e/config/oidc-standing/. Do + # not turn this preset into a denylist without re-homing them first. + # + # The role that suite's token is exchanged for is NOT standing: `formae + # connect` provisions it per run as formae-e2e-oidc-connect-role, which is + # inside the allowlist and inside the leftover-role purge above, so a run + # that dies before its teardown is reclaimed rather than accumulating. filters: __global__: - property: Name diff --git a/tests/e2e/config/oidc-standing/oidc_standing.pkl b/tests/e2e/config/oidc-standing/oidc_standing.pkl index a9940d3d0..69818016a 100644 --- a/tests/e2e/config/oidc-standing/oidc_standing.pkl +++ b/tests/e2e/config/oidc-standing/oidc_standing.pkl @@ -4,7 +4,16 @@ * SPDX-License-Identifier: FSL-1.1-ALv2 */ -/// The standing resources the oidc-credential e2e exchanges a token against. +/// The standing resources the oidc-credential e2e mints and exchanges a token +/// against: the static issuer, its published keys, and the IAM identity +/// provider that trusts it. +/// +/// The role the token is exchanged for is deliberately NOT here. `formae +/// connect` provisions it on every run, from the coordinates the e2e's stub +/// control plane produces, which is how the suite proves the connect path +/// instead of assuming its output. The identity provider stays standing +/// because it is account-global and shared, and because connect validates and +/// reuses an existing one rather than needing to create it. /// /// These are NOT created by the test run: the e2e assumes they already exist /// in the e2e account, and the aws-nuke pre-cleanup preserves them (their @@ -33,7 +42,6 @@ import "@formae/formae.pkl" import "@aws/aws.pkl" import "@aws/iam/oidcprovider.pkl" -import "@aws/iam/role.pkl" import "@aws/s3/bucket.pkl" import "@aws/s3/bucketpolicy.pkl" import "@aws/s3/object.pkl" @@ -57,11 +65,12 @@ local issuerUrl = "https://\(issuerHost)" /// Pinned in the JWKS and in the header of every minted token. local keyId = "e2e-oidc-key-1" -/// The only subject the role trusts, and the only audience it accepts. -local subject = "e2e-oidc-subject" +/// The audience the provider accepts. The subject the role trusts is not +/// here: the role is provisioned per run by `formae connect`, from the subject +/// the e2e's stub control plane hands it, which is what makes the e2e prove +/// the connect path rather than assume it. local audience = "sts.amazonaws.com" -local assumeRoleName = "e2e-oidc-assume-role" local secretName = "e2e-oidc-signing-key" /// SHA-1 fingerprint of Amazon Root CA 1, which terminates the chain for every @@ -152,33 +161,6 @@ local issuerProvider = new oidcprovider.OIDCProvider { thumbprintList = new Listing { issuerThumbprint } } -/// The role the e2e assumes. It grants nothing: what the test proves is that -/// STS accepts the token and hands back credentials, not what they can do. -local assumeRole = new role.Role { - label = "e2e-oidc-assume-role" - roleName = assumeRoleName - description = "Assumed by the oidc-credential e2e via web identity" - maxSessionDuration = 3600 - assumeRolePolicyDocument = new Dynamic { - ["Version"] = "2012-10-17" - ["Statement"] = new Listing { - new Dynamic { - ["Effect"] = "Allow" - ["Principal"] = new Dynamic { - ["Federated"] = "arn:aws:iam::\(accountId):oidc-provider/\(issuerHost)" - } - ["Action"] = "sts:AssumeRoleWithWebIdentity" - ["Condition"] = new Dynamic { - ["StringEquals"] = new Dynamic { - ["\(issuerHost):sub"] = subject - ["\(issuerHost):aud"] = audience - } - } - } - } - } -} - /// The private half, read from the agent's environment so it never lands in /// the tree. Opaque and set-once: formae writes it at create and thereafter /// neither reads it back nor diffs it. @@ -192,7 +174,7 @@ local signingKey = new secret.Secret { forma { new formae.Stack { label = "e2e-oidc-standing" - description = "Standing issuer, provider, role and key for the oidc-credential e2e" + description = "Standing issuer, provider and key for the oidc-credential e2e" } new formae.Target { @@ -207,6 +189,5 @@ forma { discoveryDocument jwks issuerProvider - assumeRole signingKey } diff --git a/tests/e2e/go/connect.go b/tests/e2e/go/connect.go new file mode 100644 index 000000000..4f2a0d5ba --- /dev/null +++ b/tests/e2e/go/connect.go @@ -0,0 +1,321 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +//go:build e2e + +package e2e_test + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/iam" +) + +// The hosted half of a `formae connect` run, stubbed. +// +// Connect cannot reach a cloud account without one. Every path opens a session +// against the control plane before it touches AWS: it resolves a hosted +// profile, drives that profile's auth plugin for a bearer, reads the subject, +// role name and issuer it must provision for, and reports the role it made +// back. This file stands up that control plane, the profile that addresses it, +// and the environment that points the CLI at both. Everything on the AWS side +// of the run — the caller check, the OIDC provider, the role, its trust policy +// — is real. + +// oidcAuthStubBearer is the credential the staged auth plugin hands back. +// Its source of truth is tests/e2e/go/fixtures/oidc-auth-stub; the stub +// control plane compares against it, so a request that did not come through +// the auth plugin is refused rather than served. +const oidcAuthStubBearer = "Bearer e2e-oidc-connect-token" + +// connectInstallationID is the installation a connect run addresses. The +// profile loader requires 27 base62 characters. Nothing resolves it: the stub +// answers for whichever installation the run names. +const connectInstallationID = "2eOidcConnectE2eInstall0001" + +// oidcAuthPluginDirEnv names the plugin tree holding the stub auth plugin, +// staged by `make test-e2e`. +const oidcAuthPluginDirEnv = "E2E_OIDC_AUTH_PLUGIN_DIR" + +// connectSetup is the coordinates the control plane produces and connect +// provisions against, verbatim: they travel from this struct into the role's +// name and its trust policy without connect inventing anything of its own. +type connectSetup struct { + CloudSubject string `json:"cloudSubject"` + CloudRoleName string `json:"cloudRoleName"` + Issuer string `json:"issuer"` +} + +// stubRegistration is one registration the stub control plane received. The +// coordinate fields mirror the CLI's own registration body, so a test can +// assert on what actually crossed the wire. +type stubRegistration struct { + Cloud string `json:"cloud"` + Account string `json:"account"` + RoleArn string `json:"roleArn"` + WorkloadIdentityProvider string `json:"workloadIdentityProvider"` +} + +// connectStub is a running stub control plane. +type connectStub struct { + URL string + + mu sync.Mutex + registrations []stubRegistration +} + +// Registrations returns what the stub received, as a copy: a test reads it +// after the run rather than racing the handler for it. +func (s *connectStub) Registrations() []stubRegistration { + s.mu.Lock() + defer s.mu.Unlock() + return append([]stubRegistration(nil), s.registrations...) +} + +// StartConnectStub serves the two endpoints a connect run calls, on loopback +// http — which the CLI's origin rule admits for exactly this reason. Only the +// endpoints a successful run reaches are served: anything else 404s, which +// surfaces an unexpected call as a failure instead of a plausible answer. +func StartConnectStub(t *testing.T, setup connectSetup) *connectStub { + t.Helper() + + stub := &connectStub{} + mux := http.NewServeMux() + + mux.HandleFunc("GET /api/v1/installations/{installation}/cloud-connection-setup", + func(w http.ResponseWriter, r *http.Request) { + if !stubAuthorized(t, w, r) { + return + } + writeStubJSON(t, w, http.StatusOK, setup) + }) + + mux.HandleFunc("POST /api/v1/installations/{installation}/cloud-connections", + func(w http.ResponseWriter, r *http.Request) { + if !stubAuthorized(t, w, r) { + return + } + var registration stubRegistration + if err := json.NewDecoder(r.Body).Decode(®istration); err != nil { + t.Errorf("stub control plane: registration body is not JSON: %v", err) + w.WriteHeader(http.StatusBadRequest) + return + } + stub.mu.Lock() + stub.registrations = append(stub.registrations, registration) + stub.mu.Unlock() + // The created row is the registration echoed back, which the CLI + // does not parse; the status is the whole of the answer. + writeStubJSON(t, w, http.StatusCreated, registration) + }) + + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + stub.URL = server.URL + return stub +} + +// stubAuthorized refuses anything that did not arrive with the credential the +// auth plugin mints. Sending 401 rather than failing the test outright keeps +// the CLI's own refusal the thing under test. +func stubAuthorized(t *testing.T, w http.ResponseWriter, r *http.Request) bool { + t.Helper() + + if r.Header.Get("Authorization") != oidcAuthStubBearer { + writeStubJSON(t, w, http.StatusUnauthorized, map[string]any{ + "error": map[string]any{"code": "unauthorized"}, + }) + return false + } + return true +} + +func writeStubJSON(t *testing.T, w http.ResponseWriter, status int, body any) { + t.Helper() + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + if err := json.NewEncoder(w).Encode(body); err != nil { + t.Errorf("stub control plane: writing the response: %v", err) + } +} + +// hostedConnectConfig is the profile a connect run is pointed at with +// --config. It is the shape `formae login` writes, plus a pluginDir so the +// staged auth plugin is the one resolved: the CLI matches an auth plugin by +// the type the auth block names, and the connect gate admits only "oidc". +// +// endpoint addresses the installation's agent and is never dialled by a +// connect run, but it has to parse as an https origin, so it names one that +// cannot resolve rather than one that could. +const hostedConnectConfig = `/* + * Auto-generated e2e test configuration + */ + +amends "formae:/Config.pkl" + +pluginDir = %q + +cli { + connection = new Hosted { + endpoint = "https://e2e-oidc-connect.invalid" + installation = %q + auth = new Dynamic { + type = "oidc" + role = "cli" + issuer = %q + clientId = "formae-cli" + scopes = "openid profile email offline_access" + } + } +} +` + +// WriteHostedConnectConfig writes the profile and returns its path. issuer is +// the login issuer the profile's auth block names, which the gate requires to +// be the platform's own: the stub serves both, so it is the stub's origin. +func WriteHostedConnectConfig(t *testing.T, dir, authPluginDir, issuer string) string { + t.Helper() + + path := filepath.Join(dir, "connect-config.pkl") + content := fmt.Sprintf(hostedConnectConfig, authPluginDir, connectInstallationID, issuer) + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write the connect config: %v", err) + } + return path +} + +// ConnectEnv is the environment a connect run needs, on top of the test +// process's own. +// +// Two override pairs, and each half of each must be set with the other. The +// control-plane pair aims the bearer: FORMAE_CLOUD_URL is where the request +// goes and FORMAE_CLOUD_ISSUER is the issuer the profile's auth block must +// name to be allowed to produce one. The connect pair pins the AWS-side trust +// artifacts: FORMAE_CONNECT_ISSUER must equal the issuer the control plane +// names, or the run refuses it as untrusted, and FORMAE_CONNECT_TEMPLATE_BASE +// only rides along, since the direct-provision path fetches no template. +func ConnectEnv(controlPlane, connectIssuer string) []string { + return append(os.Environ(), + "FORMAE_CLOUD_URL="+controlPlane, + "FORMAE_CLOUD_ISSUER="+controlPlane, + "FORMAE_CONNECT_ISSUER="+connectIssuer, + "FORMAE_CONNECT_TEMPLATE_BASE=https://formae-connect-templates.s3.us-east-1.amazonaws.com", + ) +} + +// RegisteredDocument is the machine-protocol document a connect run emits when +// a registration happened. +// Each cloud reports its own trust coordinate and omits the others, so a +// reader takes Cloud first and then the field that cloud carries. +type RegisteredDocument struct { + SchemaVersion int `json:"schemaVersion"` + Phase string `json:"phase"` + Status string `json:"status"` + Cloud string `json:"cloud"` + Account string `json:"account"` + RoleArn string `json:"roleArn"` + WorkloadIdentityProvider string `json:"workloadIdentityProvider"` + Warnings []string `json:"warnings"` +} + +// RunConnect runs the connect command with the given environment and returns +// the registration document it emitted. Machine output is the surface a test +// can assert on: it is a declared protocol rather than rendered prose. +func RunConnect(t *testing.T, bin string, env []string, args ...string) RegisteredDocument { + t.Helper() + + full := append([]string{"connect"}, args...) + full = append(full, "--output-consumer", "machine", "--output-schema", "json") + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + cmd := exec.CommandContext(ctx, bin, full...) + cmd.Env = env + var stderr bytes.Buffer + cmd.Stderr = &stderr + stdout, err := cmd.Output() + if err != nil { + t.Fatalf("formae connect failed: %v\nargs: %v\nstdout: %s\nstderr: %s", + err, full, string(stdout), stderr.String()) + } + + var doc RegisteredDocument + if err := json.Unmarshal(stdout, &doc); err != nil { + t.Fatalf("failed to parse the connect document: %v\nstdout: %s", err, string(stdout)) + } + return doc +} + +// DeleteIAMRole removes a role connect provisioned, along with the permission +// posture it attached: IAM refuses to delete a role that still carries +// policies, so they go first. Every step tolerates an absent target, because +// cleanup also runs after a failure that got partway. +func DeleteIAMRole(t *testing.T, roleName string) { + t.Helper() + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + + cfg, err := config.LoadDefaultConfig(ctx, config.WithRegion("us-west-2")) + if err != nil { + t.Errorf("cleanup: failed to load AWS config: %v", err) + return + } + client := iam.NewFromConfig(cfg) + + attached, err := client.ListAttachedRolePolicies(ctx, &iam.ListAttachedRolePoliciesInput{ + RoleName: aws.String(roleName), + }) + if err != nil { + if strings.Contains(err.Error(), "NoSuchEntity") { + return + } + t.Errorf("cleanup: listing attached policies of %q: %v", roleName, err) + return + } + for _, policy := range attached.AttachedPolicies { + if _, err := client.DetachRolePolicy(ctx, &iam.DetachRolePolicyInput{ + RoleName: aws.String(roleName), + PolicyArn: policy.PolicyArn, + }); err != nil { + t.Errorf("cleanup: detaching %s from %q: %v", aws.ToString(policy.PolicyArn), roleName, err) + } + } + + inline, err := client.ListRolePolicies(ctx, &iam.ListRolePoliciesInput{ + RoleName: aws.String(roleName), + }) + if err != nil { + t.Errorf("cleanup: listing inline policies of %q: %v", roleName, err) + return + } + for _, name := range inline.PolicyNames { + if _, err := client.DeleteRolePolicy(ctx, &iam.DeleteRolePolicyInput{ + RoleName: aws.String(roleName), + PolicyName: aws.String(name), + }); err != nil { + t.Errorf("cleanup: deleting inline policy %s of %q: %v", name, roleName, err) + } + } + + if _, err := client.DeleteRole(ctx, &iam.DeleteRoleInput{RoleName: aws.String(roleName)}); err != nil { + t.Errorf("cleanup: deleting role %q: %v", roleName, err) + } +} diff --git a/tests/e2e/go/fixtures/oidc-auth-stub/formae-plugin.pkl b/tests/e2e/go/fixtures/oidc-auth-stub/formae-plugin.pkl new file mode 100644 index 000000000..c48860385 --- /dev/null +++ b/tests/e2e/go/fixtures/oidc-auth-stub/formae-plugin.pkl @@ -0,0 +1,16 @@ +/* + * © 2026 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +/// Installed as `oidc`, not as this directory's name: the CLI matches an auth +/// plugin by the `type` its profile's auth block names, and the connect gate +/// admits only "oidc". The staging step in the Makefile lays it out under +/// that name. +name = "oidc" +type = "auth" +version = "0.0.1" +summary = "Stub hosted auth plugin for the e2e suite" +license = "FSL-1.1-ALv2" +category = "auth" diff --git a/tests/e2e/go/fixtures/oidc-auth-stub/go.mod b/tests/e2e/go/fixtures/oidc-auth-stub/go.mod new file mode 100644 index 000000000..4e8acafba --- /dev/null +++ b/tests/e2e/go/fixtures/oidc-auth-stub/go.mod @@ -0,0 +1,7 @@ +module github.com/platform-engineering-labs/formae/tests/e2e/go/fixtures/oidc-auth-stub + +go 1.26 + +replace github.com/platform-engineering-labs/formae/pkg/auth => ../../../../../pkg/auth + +require github.com/platform-engineering-labs/formae/pkg/auth v0.0.0 diff --git a/tests/e2e/go/fixtures/oidc-auth-stub/main.go b/tests/e2e/go/fixtures/oidc-auth-stub/main.go new file mode 100644 index 000000000..0a92b4d72 --- /dev/null +++ b/tests/e2e/go/fixtures/oidc-auth-stub/main.go @@ -0,0 +1,51 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +// Command oidc-auth-stub stands in for the hosted auth plugin so the e2e +// suite can drive `formae connect` against a control plane of its own. +// +// It is installed under the name `oidc` because that is what the CLI looks +// for: a hosted profile's auth block names the plugin in its `type` field, +// and the connect gate requires that name exactly before it will send a +// credential anywhere. The real plugin mints its bearer through an +// interactive sign-in against a deployed issuer, which no CI run can carry +// out; this one hands back a fixed token the e2e's stub control plane +// recognises, so what the test exercises is the connect flow rather than the +// sign-in behind it. +package main + +import ( + "github.com/platform-engineering-labs/formae/pkg/auth" +) + +// Bearer is the credential this plugin hands back. The e2e's stub control +// plane compares against it byte for byte, so a run that reached the control +// plane with anything else did not come through the auth plugin. +const Bearer = "Bearer e2e-oidc-connect-token" + +// stub answers GetAuthHeader and nothing else. Validate is what an agent +// would call to check an inbound credential, and no agent in this suite is +// configured with this plugin, so the embedded base's unsupported answer is +// the honest one. +type stub struct { + auth.UnimplementedAuthPlugin +} + +// Init accepts whatever the profile's auth block carries. The block is the +// oidc plugin's CLI configuration and this stub reads nothing out of it: the +// gate has already checked the fields that decide whether a credential may +// be minted at all. +func (s *stub) Init(_ *auth.InitRequest, _ *auth.InitResponse) error { return nil } + +// GetAuthHeader returns the fixed credential under the canonical header key. +// The client attaches only "Authorization", so returning it under any other +// spelling would fail closed. +func (s *stub) GetAuthHeader(_ *auth.GetAuthHeaderRequest, resp *auth.GetAuthHeaderResponse) error { + resp.Headers = map[string][]string{"Authorization": {Bearer}} + return nil +} + +func main() { + auth.Run(&stub{}) +} diff --git a/tests/e2e/go/fixtures/oidc-credential-stub/main.go b/tests/e2e/go/fixtures/oidc-credential-stub/main.go index b6ed4e2ef..2977ea671 100644 --- a/tests/e2e/go/fixtures/oidc-credential-stub/main.go +++ b/tests/e2e/go/fixtures/oidc-credential-stub/main.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "log" + "os" "time" "github.com/aws/aws-sdk-go-v2/aws" @@ -38,9 +39,12 @@ const ( // it from the token header to pick the public key it verifies with. keyID = "e2e-oidc-key-1" - // subject is the `sub` claim. The assumed role's trust policy conditions - // on this value. - subject = "e2e-oidc-subject" + // subjectEnv names the environment variable carrying the `sub` claim. + // The subject is produced by the control plane, travels through `formae + // connect` into the trust it provisions, and has to be the same string + // here or nothing the broker mints is accepted. The test owns both ends, + // so it supplies it rather than this file pinning a copy. + subjectEnv = "E2E_OIDC_SUBJECT" // signingKeySecretID names the Secrets Manager secret whose SecretString // is the PEM-encoded RSA private key matching keyID in the JWKS. @@ -56,6 +60,7 @@ const ( type stub struct { signingKey *rsa.PrivateKey + subject string } // Configure fetches the signing key before the broker starts serving. The @@ -65,6 +70,14 @@ type stub struct { func (s *stub) Configure(_ json.RawMessage) error { ctx := context.Background() + // Read before the key: a broker with no subject can sign perfectly well + // and still mint a token nothing will accept, which surfaces as an opaque + // rejection at the far end rather than as the missing configuration it is. + s.subject = os.Getenv(subjectEnv) + if s.subject == "" { + return fmt.Errorf("%s is not set, so the broker has no subject to mint for", subjectEnv) + } + cfg, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion(awsRegion)) if err != nil { return fmt.Errorf("loading aws config: %w", err) @@ -126,7 +139,7 @@ func (s *stub) IdentityToken(_ context.Context, req *credential.OidcIdentityToke token := jwt.NewWithClaims(jwt.SigningMethodRS256, jwt.MapClaims{ "iss": issuer, - "sub": subject, + "sub": s.subject, "aud": req.Audience, "iat": jwt.NewNumericDate(now), "exp": jwt.NewNumericDate(expiresAt), diff --git a/tests/e2e/go/fixtures/oidc-echo-plugin/plugin.go b/tests/e2e/go/fixtures/oidc-echo-plugin/plugin.go index 3de6dd528..3a2850f1e 100644 --- a/tests/e2e/go/fixtures/oidc-echo-plugin/plugin.go +++ b/tests/e2e/go/fixtures/oidc-echo-plugin/plugin.go @@ -5,9 +5,13 @@ package main import ( + "bytes" "context" "encoding/json" "fmt" + "io" + "net/http" + "os" "time" "github.com/aws/aws-sdk-go-v2/aws" @@ -18,27 +22,72 @@ import ( "github.com/platform-engineering-labs/formae/pkg/plugin/resource" ) +// Every coordinate this fixture needs comes from the environment the agent +// was started with, because none of them exist until the run that provisions +// them. `formae connect` makes the trust, and only then is there an audience +// to mint for or a role to assume. const ( - // audience the plugin asks its broker for, and the audience the role's - // trust policy expects on the token it is handed. - audience = "sts.amazonaws.com" + // audienceEnv names the audience the plugin asks its broker to mint for. + // A token carries exactly one, and each cloud spells it differently: AWS + // wants sts.amazonaws.com, GCP wants the workload identity provider's own + // resource name. + audienceEnv = "E2E_OIDC_AUDIENCE" + + // assumeRoleARNEnv and gcpProjectEnv each select one cloud's exchange, and + // carry the one coordinate that exchange needs beyond the audience. + assumeRoleARNEnv = "E2E_OIDC_ASSUME_ROLE_ARN" + gcpProjectEnv = "E2E_OIDC_GCP_PROJECT" // namespace this plugin serves, as its manifest declares it. Named in the // recorded failure so a test can tell whose pairing was missing. namespace = "OidcEcho" - // assumeRoleARN is the standing role the minted token is exchanged for. - // Its trust policy names the broker's issuer, subject, and audience. - assumeRoleARN = "arn:aws:iam::942849037363:role/e2e-oidc-assume-role" - // assumeRoleSessionName labels the STS session. Stable, so the assumed // role ARN a test reads back is stable too. assumeRoleSessionName = "e2e-oidc-echo" - // awsRegion is where the STS call is made. + // awsRegion is where the AWS STS call is made. awsRegion = "us-west-2" ) +// exchange is the cloud-specific half of the probe: what the token is spent +// on once the broker has minted it. +// +// An agent runs exactly one, chosen by which coordinates the test put in its +// environment, because a token is minted for one audience and is accepted by +// one exchange only. +type exchange interface { + // Spend exchanges the identity token for real credentials and reports + // proof of it: who the caller became, and how long the credentials last. + // + // The credentials themselves are deliberately not returned. They would + // land in the resource's properties, which is not a place credentials + // belong. + Spend(ctx context.Context, token string) (identity, expiration string, err error) +} + +// resolveExchange picks the exchange the environment configured. Naming both +// variables in the failure matters: the symptom of setting neither is a token +// that was minted and then not spent, which looks like the exchange failing +// rather than never having been asked for. +func resolveExchange() (exchange, error) { + roleARN := os.Getenv(assumeRoleARNEnv) + project := os.Getenv(gcpProjectEnv) + + switch { + case roleARN != "" && project != "": + return nil, fmt.Errorf("%s and %s are both set, so which cloud to exchange at is ambiguous", + assumeRoleARNEnv, gcpProjectEnv) + case roleARN != "": + return awsExchange{roleARN: roleARN}, nil + case project != "": + return gcpExchange{audience: os.Getenv(audienceEnv), project: project}, nil + default: + return nil, fmt.Errorf("neither %s nor %s is set, so there is nothing to exchange the token at", + assumeRoleARNEnv, gcpProjectEnv) + } +} + // EchoPlugin serves OidcEcho::Tokens::Token. It holds no state beyond the // token source the SDK installs, which resolves the paired broker per call. type EchoPlugin struct { @@ -64,16 +113,18 @@ func (p *EchoPlugin) LabelConfig() pkgmodel.LabelConfig { return pkgmodel.LabelConfig{DefaultQuery: "$.probeLabel"} } -// tokenProperties asks the broker for a token, exchanges it at AWS STS, and -// renders the resource's properties around both outcomes. A failure is -// recorded in tokenError or stsError rather than failed outright, so a test -// asserting the unpaired case reads an exact message instead of whatever an -// operator error renders to. Real plugins fail closed here; this one is only -// proving the wiring. +// tokenProperties asks the broker for a token, spends it at the configured +// exchange, and renders the resource's properties around both outcomes. A +// failure is recorded in tokenError or exchangeError rather than failed +// outright, so a test asserting the unpaired case reads an exact message +// instead of whatever an operator error renders to. Real plugins fail closed +// here; this one is only proving the wiring. func (p *EchoPlugin) tokenProperties(ctx context.Context, probeLabel string) (json.RawMessage, error) { var token, tokenError string - switch { + switch audience := os.Getenv(audienceEnv); { + case audience == "": + tokenError = fmt.Sprintf("%s is not set, so there is no audience to mint for", audienceEnv) case p.tokens == nil: // The SDK installs the source on every OidcAware plugin, so this only // fires if that wiring broke. @@ -87,51 +138,95 @@ func (p *EchoPlugin) tokenProperties(ctx context.Context, probeLabel string) (js } } - // With no token there is nothing to exchange, so the STS outputs stay + // With no token there is nothing to spend, so the exchange outputs stay // empty rather than carrying the error of a call that was never worth // making. - var assumedRoleARN, expiration, stsError string + var identity, expiration, exchangeError string if token != "" { - assumedRoleARN, expiration, stsError = assumeRoleWithToken(ctx, token) + identity, expiration, exchangeError = spend(ctx, token) } return json.Marshal(map[string]string{ - "probeLabel": probeLabel, - "token": token, - "tokenError": tokenError, - "stsAssumedRoleArn": assumedRoleARN, - "stsExpiration": expiration, - "stsError": stsError, + "probeLabel": probeLabel, + "token": token, + "tokenError": tokenError, + "exchangeIdentity": identity, + "exchangeExpiration": expiration, + "exchangeError": exchangeError, }) } -// assumeRoleWithToken exchanges an identity token for role credentials at -// AWS STS and reports proof of the exchange: who the caller became and how -// long the credentials last. The access key and secret are deliberately not -// returned: they would land in the resource's properties, which is not a -// place credentials belong. -func assumeRoleWithToken(ctx context.Context, token string) (assumedRoleARN, expiration, stsError string) { - cfg, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion(awsRegion)) +// spend runs the configured exchange and flattens its outcome into the three +// strings the properties document carries. +func spend(ctx context.Context, token string) (identity, expiration, exchangeError string) { + ex, err := resolveExchange() if err != nil { - return "", "", fmt.Sprintf("loading aws config: %s", err) + return "", "", err.Error() } - - out, err := sts.NewFromConfig(cfg).AssumeRoleWithWebIdentity(ctx, &sts.AssumeRoleWithWebIdentityInput{ - RoleArn: aws.String(assumeRoleARN), - RoleSessionName: aws.String(assumeRoleSessionName), - WebIdentityToken: aws.String(token), - }) + identity, expiration, err = ex.Spend(ctx, token) if err != nil { - return "", "", fmt.Sprintf("assume role with web identity: %s", err) + return "", "", err.Error() } + return identity, expiration, "" +} + +// awsExchange trades the identity token for role credentials at AWS STS. The +// role is the one `formae connect` provisioned, and its trust policy pins the +// broker's issuer, subject and audience, so a token STS accepts here is proof +// the whole chain agrees. +type awsExchange struct { + roleARN string +} + +// assumeRolePropagation bounds how long the exchange keeps trying a role that +// is not assumable yet, and how often. +// +// A role created moments ago is not immediately assumable: IAM propagates +// asynchronously and STS answers AccessDenied in the meantime. That is the +// ordinary state of a role `formae connect` made seconds earlier, so reporting +// the first refusal would fail the run over a trust policy that is correct. +// +// The window is well inside the agent's 60s plugin call deadline, so this +// gives up before the operator does and reports STS's own last refusal. A +// genuinely wrong trust policy still fails, at the end of the window rather +// than at the start of it. +const ( + assumeRolePropagationWindow = 40 * time.Second + assumeRolePropagationInterval = 2 * time.Second +) - if out.AssumedRoleUser != nil && out.AssumedRoleUser.Arn != nil { - assumedRoleARN = *out.AssumedRoleUser.Arn +func (e awsExchange) Spend(ctx context.Context, token string) (identity, expiration string, err error) { + cfg, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion(awsRegion)) + if err != nil { + return "", "", fmt.Errorf("loading aws config: %w", err) } - if out.Credentials != nil && out.Credentials.Expiration != nil { - expiration = out.Credentials.Expiration.Format(time.RFC3339) + client := sts.NewFromConfig(cfg) + + deadline := time.Now().Add(assumeRolePropagationWindow) + for { + out, err := client.AssumeRoleWithWebIdentity(ctx, &sts.AssumeRoleWithWebIdentityInput{ + RoleArn: aws.String(e.roleARN), + RoleSessionName: aws.String(assumeRoleSessionName), + WebIdentityToken: aws.String(token), + }) + if err == nil { + if out.AssumedRoleUser != nil && out.AssumedRoleUser.Arn != nil { + identity = *out.AssumedRoleUser.Arn + } + if out.Credentials != nil && out.Credentials.Expiration != nil { + expiration = out.Credentials.Expiration.Format(time.RFC3339) + } + return identity, expiration, nil + } + if time.Now().After(deadline) { + return "", "", fmt.Errorf("assume role with web identity: %w", err) + } + select { + case <-ctx.Done(): + return "", "", fmt.Errorf("assume role with web identity: %w", ctx.Err()) + case <-time.After(assumeRolePropagationInterval): + } } - return assumedRoleARN, expiration, "" } // probeLabelOf reads the probeLabel property out of a properties document. @@ -243,3 +338,149 @@ func failure(operation resource.Operation, err error) *resource.ProgressResult { StatusMessage: err.Error(), } } + +// gcpExchange trades the identity token for a federated access token at +// Google's STS, then spends that token reading the project. +// +// The two steps are both needed and prove different things. The exchange +// proves the workload identity provider `formae connect` created trusts the +// broker's issuer and accepts its subject and audience; the project read +// proves connect also granted that federated principal something, which is +// the half a successful exchange alone would not show. +// +// It is written against the REST endpoints rather than the Google SDK because +// the SDK's federation support wants a credential-configuration file naming a +// token source on disk, and the token here arrives in memory from the broker. +type gcpExchange struct { + // audience is the workload identity provider's full resource name, which + // is both what the token was minted for and what the exchange is + // addressed to. Google pins the provider's allowed audiences to this same + // string, so the two cannot drift. + audience string + project string +} + +// gcpTokenExchangeURL and gcpProjectURL are Google's STS token endpoint and +// the project read used as proof the exchanged credentials work. +const ( + gcpTokenExchangeURL = "https://sts.googleapis.com/v1/token" + gcpProjectURL = "https://cloudresourcemanager.googleapis.com/v1/projects/" +) + +func (e gcpExchange) Spend(ctx context.Context, token string) (identity, expiration string, err error) { + if e.audience == "" { + return "", "", fmt.Errorf("%s is not set, so there is no workload identity provider to exchange at", audienceEnv) + } + + accessToken, lifetime, err := e.federate(ctx, token) + if err != nil { + return "", "", err + } + + number, err := e.readProjectNumber(ctx, accessToken) + if err != nil { + return "", "", err + } + + // The identity is reported as the project the credentials could actually + // read, for the same reason the AWS side reports the assumed role ARN: + // a token that exchanged but reaches nothing has not proven access. + return "projects/" + number, time.Now().Add(lifetime).UTC().Format(time.RFC3339), nil +} + +// federate performs the RFC 8693 token exchange and returns the access token +// and how long it lasts. +func (e gcpExchange) federate(ctx context.Context, token string) (string, time.Duration, error) { + body, err := json.Marshal(map[string]string{ + "audience": e.audience, + "grantType": "urn:ietf:params:oauth:grant-type:token-exchange", + "requestedTokenType": "urn:ietf:params:oauth:token-type:access_token", + "scope": "https://www.googleapis.com/auth/cloud-platform", + "subjectTokenType": "urn:ietf:params:oauth:token-type:jwt", + "subjectToken": token, + }) + if err != nil { + return "", 0, fmt.Errorf("building the token exchange request: %w", err) + } + + data, err := gcpPost(ctx, gcpTokenExchangeURL, "", body) + if err != nil { + return "", 0, fmt.Errorf("exchanging the identity token: %w", err) + } + + var exchanged struct { + AccessToken string `json:"access_token"` + ExpiresIn int `json:"expires_in"` + } + if err := json.Unmarshal(data, &exchanged); err != nil { + return "", 0, fmt.Errorf("parsing the token exchange response: %w", err) + } + if exchanged.AccessToken == "" { + return "", 0, fmt.Errorf("the token exchange returned no access token") + } + return exchanged.AccessToken, time.Duration(exchanged.ExpiresIn) * time.Second, nil +} + +// readProjectNumber spends the federated access token on the one read that +// shows it carries access, and returns what it read back. +func (e gcpExchange) readProjectNumber(ctx context.Context, accessToken string) (string, error) { + data, err := gcpGet(ctx, gcpProjectURL+e.project, accessToken) + if err != nil { + return "", fmt.Errorf("reading project %s with the exchanged credentials: %w", e.project, err) + } + + var project struct { + ProjectNumber string `json:"projectNumber"` + } + if err := json.Unmarshal(data, &project); err != nil { + return "", fmt.Errorf("parsing the project read response: %w", err) + } + if project.ProjectNumber == "" { + return "", fmt.Errorf("the project read returned no project number") + } + return project.ProjectNumber, nil +} + +func gcpPost(ctx context.Context, url, accessToken string, body []byte) ([]byte, error) { + return gcpDo(ctx, http.MethodPost, url, accessToken, body) +} + +func gcpGet(ctx context.Context, url, accessToken string) ([]byte, error) { + return gcpDo(ctx, http.MethodGet, url, accessToken, nil) +} + +// gcpDo makes one call and returns the body. A non-2xx carries Google's own +// error text, capped: the whole value of this fixture on a failing run is the +// reason Google gave, and a refusal at the exchange and a refusal at the read +// are different problems that read almost identically without it. +func gcpDo(ctx context.Context, method, url, accessToken string, body []byte) ([]byte, error) { + var reader io.Reader + if body != nil { + reader = bytes.NewReader(body) + } + req, err := http.NewRequestWithContext(ctx, method, url, reader) + if err != nil { + return nil, err + } + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + if accessToken != "" { + req.Header.Set("Authorization", "Bearer "+accessToken) + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + + data, err := io.ReadAll(io.LimitReader(resp.Body, 1<<16)) + if err != nil { + return nil, err + } + if resp.StatusCode < 200 || resp.StatusCode > 299 { + return nil, fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(data)) + } + return data, nil +} diff --git a/tests/e2e/go/oidc_credential_test.go b/tests/e2e/go/oidc_credential_test.go index 55e20ac12..8662b0fb3 100644 --- a/tests/e2e/go/oidc_credential_test.go +++ b/tests/e2e/go/oidc_credential_test.go @@ -18,15 +18,37 @@ import ( const ( // The audience the echo plugin asks for in Create, and the claims the - // stub broker signs into the token it mints for it. These name standing - // AWS resources: the static OIDC issuer registered as an IAM identity - // provider, the key in its published JWKS, and the subject and role its - // trust policy conditions on. + // stub broker signs into the token it mints for it. The issuer and key + // are standing AWS resources: a static OIDC issuer served from S3 and the + // key in its published JWKS. The subject is what the stub control plane + // hands connect, so it is also what the provisioned role's trust policy + // conditions on. oidcEchoAudience = "sts.amazonaws.com" oidcEchoIssuer = "https://e2e-oidc-issuer-942849037363.s3.us-west-2.amazonaws.com" oidcEchoKeyID = "e2e-oidc-key-1" - oidcEchoSubject = "e2e-oidc-subject" - oidcEchoRoleName = "e2e-oidc-assume-role" + + // The subject the control plane produces, in the grammar it really uses: + // the `fai:` namespace, a tenant, and the installation being connected. + // Connect takes it verbatim into the trust it provisions and the GCP path + // parses it back apart, so a fixture subject of a made-up shape would + // exercise a string production never emits. + oidcEchoTenantID = "2eOidcConnectE2eTenant00001" + oidcEchoSubject = "fai:" + oidcEchoTenantID + "/" + connectInstallationID + + // The e2e account, pinned rather than derived: connect is told which + // account to connect and never infers one from ambient credentials, and + // the test asserts on the role ARN that produces. + oidcEchoAccount = "942849037363" + + // The role connect provisions and the echo plugin then assumes. The name + // carries the suite's own prefix so the pre-cleanup purge reclaims it if + // a run dies before its teardown; a survivor is converged rather than + // refused, since it carries the ownership tags connect wrote. + oidcConnectRoleName = "formae-e2e-oidc-connect-role" + + // The AWS shared-config profile connect provisions with, written by the + // e2e workflow's credentials step. + oidcConnectAWSProfile = "e2e-test" oidcEchoStackQuery = "stack:e2e-oidc-echo" oidcEchoLabel = "e2e-oidc-token" @@ -122,6 +144,22 @@ func decodeJWT(t *testing.T, token string) (header, claims map[string]any) { return decodeJWTSegment(t, "header", segments[0]), decodeJWTSegment(t, "claims", segments[1]) } +// requireCredentialsOutlive holds the exchange's reported expiry to being a +// real one: an RFC3339 instant still in the future. It is the difference +// between an exchange that returned credentials and one that returned a +// document shaped like credentials. +func requireCredentialsOutlive(t *testing.T, expiration string) { + t.Helper() + + expiresAt, err := time.Parse(time.RFC3339, expiration) + if err != nil { + t.Fatalf("exchangeExpiration %q is not RFC3339: %v", expiration, err) + } + if !expiresAt.After(time.Now()) { + t.Errorf("exchangeExpiration %q is not in the future", expiration) + } +} + // jwtString reads a string-valued entry out of a decoded token document. func jwtString(t *testing.T, doc map[string]any, key string) string { t.Helper() @@ -137,14 +175,87 @@ func jwtString(t *testing.T, doc map[string]any, key string) string { return str } -// TestOidcCredential_TokenExchangesForRealCredentials proves the whole chain: -// the agent discovers and spawns the stub broker, pairs it with the echo -// plugin's namespace, the signed token the broker mints for the audience the -// plugin asks for arrives inside the plugin's Create, and AWS STS accepts -// that token and exchanges it for credentials on the standing role. +// connectProvisionsTrust drives `formae connect aws` against a stub control +// plane and returns the ARN of the role it provisioned. +// +// This is the half of the chain that establishes trust. Connect reads the +// subject, role name and issuer from the control plane and provisions against +// them verbatim: the OIDC provider for the standing issuer (which already +// exists, so it is validated and reused) and a role whose trust policy pins +// that provider, that subject, and the STS audience. Nothing here is +// hand-provisioned, and nothing is asserted from the CLI's prose — the +// machine document and the registration the stub received are the evidence. +func connectProvisionsTrust(t *testing.T, bin string) string { + t.Helper() + + stub := StartConnectStub(t, connectSetup{ + CloudSubject: oidcEchoSubject, + CloudRoleName: oidcConnectRoleName, + Issuer: oidcEchoIssuer, + }) + configPath := WriteHostedConnectConfig(t, t.TempDir(), + stagedOidcPluginDir(t, oidcAuthPluginDirEnv), stub.URL) + + // Registered before the run, not after it: a run that provisions the role + // and then fails to register still leaves the role standing. + t.Cleanup(func() { DeleteIAMRole(t, oidcConnectRoleName) }) + + doc := RunConnect(t, bin, ConnectEnv(stub.URL, oidcEchoIssuer), + "aws", + "--config", configPath, + "--account", oidcEchoAccount, + "--profile-aws", oidcConnectAWSProfile, + "--no-input", + ) + + if doc.Phase != "registered" { + t.Fatalf("connect phase: got %q, want %q", doc.Phase, "registered") + } + if doc.Cloud != "aws" { + t.Errorf("connect cloud: got %q, want %q", doc.Cloud, "aws") + } + if doc.Account != oidcEchoAccount { + t.Errorf("connect account: got %q, want %q", doc.Account, oidcEchoAccount) + } + if !strings.HasSuffix(doc.RoleArn, ":role/"+oidcConnectRoleName) { + t.Fatalf("connect roleArn %q does not name role %q", doc.RoleArn, oidcConnectRoleName) + } + + // What the control plane was told, rather than what the CLI printed: the + // registration is the connection, and a run that provisioned without + // declaring it has not connected the account. + registrations := stub.Registrations() + if len(registrations) != 1 { + t.Fatalf("stub control plane received %d registrations, want 1: %+v", len(registrations), registrations) + } + if got := registrations[0]; got.Cloud != "aws" || got.Account != oidcEchoAccount || got.RoleArn != doc.RoleArn { + t.Errorf("registration: got %+v, want cloud aws, account %s, roleArn %s", + got, oidcEchoAccount, doc.RoleArn) + } + + return doc.RoleArn +} + +// TestOidcCredential_TokenExchangesForRealCredentials proves the whole chain, +// from establishing trust to spending it: `formae connect` provisions the +// identity provider and the role and registers them; then the agent discovers +// and spawns the stub broker, pairs it with the echo plugin's namespace, the +// signed token the broker mints for the audience the plugin asks for arrives +// inside the plugin's Create, and AWS STS accepts that token against the role +// connect just made. func TestOidcCredential_TokenExchangesForRealCredentials(t *testing.T) { bin := FormaeBinary(t) - agent := StartAgent(t, bin, WithPluginDir(stagedOidcPluginDir(t, oidcPluginDirEnv))) + + roleArn := connectProvisionsTrust(t, bin) + + agent := StartAgent(t, bin, + WithPluginDir(stagedOidcPluginDir(t, oidcPluginDirEnv)), + WithEnv( + "E2E_OIDC_SUBJECT="+oidcEchoSubject, + "E2E_OIDC_AUDIENCE="+oidcEchoAudience, + "E2E_OIDC_ASSUME_ROLE_ARN="+roleArn, + ), + ) agent.WaitForOidcBroker(t, oidcEchoNamespace, 60*time.Second) echo := applyOidcEchoFixture(t, bin, agent) @@ -170,21 +281,13 @@ func TestOidcCredential_TokenExchangesForRealCredentials(t *testing.T) { } } - if got := echoOutput(t, echo, "stsError"); got != "" { - t.Fatalf("stsError: got %q, want empty", got) + if got := echoOutput(t, echo, "exchangeError"); got != "" { + t.Fatalf("exchangeError: got %q, want empty", got) } - if got := echoOutput(t, echo, "stsAssumedRoleArn"); !strings.Contains(got, oidcEchoRoleName) { - t.Errorf("stsAssumedRoleArn %q does not name role %q", got, oidcEchoRoleName) - } - - expiration := echoOutput(t, echo, "stsExpiration") - expiresAt, err := time.Parse(time.RFC3339, expiration) - if err != nil { - t.Fatalf("stsExpiration %q is not RFC3339: %v", expiration, err) - } - if !expiresAt.After(time.Now()) { - t.Errorf("stsExpiration %q is not in the future", expiration) + if got := echoOutput(t, echo, "exchangeIdentity"); !strings.Contains(got, oidcConnectRoleName) { + t.Errorf("exchangeIdentity %q does not name role %q", got, oidcConnectRoleName) } + requireCredentialsOutlive(t, echoOutput(t, echo, "exchangeExpiration")) } // TestOidcCredential_NoBrokerFailsClosed proves a plugin whose namespace has @@ -193,7 +296,12 @@ func TestOidcCredential_TokenExchangesForRealCredentials(t *testing.T) { // both the missing pairing and its own namespace. func TestOidcCredential_NoBrokerFailsClosed(t *testing.T) { bin := FormaeBinary(t) - agent := StartAgent(t, bin, WithPluginDir(stagedOidcPluginDir(t, oidcPluginDirNoBrokerEnv))) + agent := StartAgent(t, bin, + WithPluginDir(stagedOidcPluginDir(t, oidcPluginDirNoBrokerEnv)), + // An audience, but no exchange coordinates: the plugin gets far enough + // to ask for a token, which is where this test's failure has to happen. + WithEnv("E2E_OIDC_AUDIENCE="+oidcEchoAudience), + ) echo := applyOidcEchoFixture(t, bin, agent) From 10f100bed174236253147f899f5cd690d215a11a Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Thu, 27 Aug 2026 17:26:37 -0700 Subject: [PATCH 2/7] test(e2e): add the GCP sibling of the oidc-credential chain The same property against a different federation mechanism: connect provisions the workload identity pool, the provider trusting the broker's issuer, and the project bindings; the broker mints for the provider's own resource name; Google's STS exchanges that for a federated access token; and the token reads the project. The read is not decoration. An exchange proves the provider trusts the issuer, subject and audience, and only spending its result proves connect granted the federated principal anything, which is the half a successful exchange alone leaves unchecked. Nothing torn down afterwards, deliberately: everything connect creates here converges rather than accumulates. The pool and provider are fixed per project and patched in place, and the bindings are set for one principal that does not change between runs. It skips until a project is named, and nothing names one yet. provx/gcp resolves its issuer from a compile-time constant instead of the one its caller passes, so connect provisions trust for the production issuer no matter which issuer the control plane named and the exchange could never succeed. That fix is in flight upstream; wiring the project in belongs with the dependency bump that carries it. --- tests/e2e/go/oidc_credential_gcp_test.go | 165 +++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 tests/e2e/go/oidc_credential_gcp_test.go diff --git a/tests/e2e/go/oidc_credential_gcp_test.go b/tests/e2e/go/oidc_credential_gcp_test.go new file mode 100644 index 000000000..c543c7307 --- /dev/null +++ b/tests/e2e/go/oidc_credential_gcp_test.go @@ -0,0 +1,165 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +//go:build e2e + +package e2e_test + +import ( + "os" + "strings" + "testing" + "time" +) + +// gcpProjectEnv names the project the GCP half of the oidc-credential chain +// federates into. It is its own project rather than the one the GCP plugin's +// suites use, because provx fixes the workload identity pool and provider ids +// per project and refuses to converge a provider that trusts a different +// issuer: a project already carrying the production-issuer connection cannot +// also carry this one. +const gcpProjectEnv = "E2E_GCP_PROJECT" + +// gcpE2EProject returns the project, or skips. +// +// A skip rather than a failure because the credentials are conditional in the +// workflow, the way Azure's are: a fork without them should report that this +// did not run, not that it broke. The message names the variable, so a skip in +// a run that was supposed to have credentials is legible as the misconfigured +// job it is. +func gcpE2EProject(t *testing.T) string { + t.Helper() + + project := os.Getenv(gcpProjectEnv) + if project == "" { + t.Skipf("%s is not set, so there is no GCP project to federate into", gcpProjectEnv) + } + return project +} + +// connectProvisionsGcpTrust drives `formae connect gcp` against a stub control +// plane and returns the workload identity provider it provisioned. +// +// Nothing is torn down afterwards, deliberately. Everything connect creates +// here converges rather than accumulates: the pool and the provider are fixed +// per project and are patched in place, and the IAM bindings are set for one +// principal that does not change between runs. provx's own Delete leaves the +// pool and provider standing for the same reason — they are shared — so a +// teardown here would remove two bindings the next run puts straight back. +func connectProvisionsGcpTrust(t *testing.T, bin, project string) string { + t.Helper() + + stub := StartConnectStub(t, connectSetup{ + CloudSubject: oidcEchoSubject, + // GCP carries no role, but the setup read requires all three + // coordinates: the control plane serves one document for every cloud. + CloudRoleName: oidcConnectRoleName, + Issuer: oidcEchoIssuer, + }) + configPath := WriteHostedConnectConfig(t, t.TempDir(), + stagedOidcPluginDir(t, oidcAuthPluginDirEnv), stub.URL) + + doc := RunConnect(t, bin, ConnectEnv(stub.URL, oidcEchoIssuer), + "gcp", + "--config", configPath, + "--project", project, + "--no-input", + ) + + if doc.Phase != "registered" { + t.Fatalf("connect phase: got %q, want %q", doc.Phase, "registered") + } + if doc.Cloud != "gcp" { + t.Errorf("connect cloud: got %q, want %q", doc.Cloud, "gcp") + } + if doc.Account != project { + t.Errorf("connect account: got %q, want %q", doc.Account, project) + } + if doc.RoleArn != "" { + t.Errorf("connect reported a roleArn %q on a GCP registration", doc.RoleArn) + } + if !strings.HasPrefix(doc.WorkloadIdentityProvider, "//iam.googleapis.com/projects/") { + t.Fatalf("connect workloadIdentityProvider %q is not a provider resource name", doc.WorkloadIdentityProvider) + } + + registrations := stub.Registrations() + if len(registrations) != 1 { + t.Fatalf("stub control plane received %d registrations, want 1: %+v", len(registrations), registrations) + } + got := registrations[0] + if got.Cloud != "gcp" || got.Account != project || got.WorkloadIdentityProvider != doc.WorkloadIdentityProvider { + t.Errorf("registration: got %+v, want cloud gcp, account %s, workloadIdentityProvider %s", + got, project, doc.WorkloadIdentityProvider) + } + // The AWS coordinate is not merely empty on the wire, it is absent: the + // control plane's schema is a discriminated union that rejects a field + // belonging to another variant. + if got.RoleArn != "" { + t.Errorf("registration carried a roleArn %q on a GCP connection", got.RoleArn) + } + + return doc.WorkloadIdentityProvider +} + +// TestOidcCredential_GcpTokenExchangesForRealCredentials is the GCP sibling of +// the AWS chain, and proves the same property against a different federation +// mechanism: `formae connect` provisions the workload identity pool, the +// provider trusting the broker's issuer, and the project bindings; then the +// broker mints a token for the provider's own resource name, Google's STS +// exchanges it for a federated access token, and that token reads the project. +// +// The project read is not decoration. A token exchange proves the provider +// trusts the issuer, subject and audience; only spending the result proves +// connect also granted the federated principal something, which is the half +// the exchange alone would leave unchecked. +func TestOidcCredential_GcpTokenExchangesForRealCredentials(t *testing.T) { + bin := FormaeBinary(t) + project := gcpE2EProject(t) + + provider := connectProvisionsGcpTrust(t, bin, project) + + agent := StartAgent(t, bin, + WithPluginDir(stagedOidcPluginDir(t, oidcPluginDirEnv)), + WithEnv( + "E2E_OIDC_SUBJECT="+oidcEchoSubject, + // On GCP the audience is the provider's own resource name, and + // Google pins the provider's allowed audiences to that same + // string, so the token and the exchange cannot disagree. + "E2E_OIDC_AUDIENCE="+provider, + "E2E_OIDC_GCP_PROJECT="+project, + ), + ) + agent.WaitForOidcBroker(t, oidcEchoNamespace, 60*time.Second) + + echo := applyOidcEchoFixture(t, bin, agent) + + if got := echoOutput(t, echo, "tokenError"); got != "" { + t.Fatalf("tokenError: got %q, want empty", got) + } + + header, claims := decodeJWT(t, echoOutput(t, echo, "token")) + for _, check := range []struct { + doc map[string]any + key string + expected string + }{ + {header, "alg", "RS256"}, + {header, "kid", oidcEchoKeyID}, + {claims, "iss", oidcEchoIssuer}, + {claims, "sub", oidcEchoSubject}, + {claims, "aud", provider}, + } { + if got := jwtString(t, check.doc, check.key); got != check.expected { + t.Errorf("token %s: got %q, want %q", check.key, got, check.expected) + } + } + + if got := echoOutput(t, echo, "exchangeError"); got != "" { + t.Fatalf("exchangeError: got %q, want empty", got) + } + if got := echoOutput(t, echo, "exchangeIdentity"); !strings.HasPrefix(got, "projects/") { + t.Errorf("exchangeIdentity %q does not name the project the credentials read", got) + } + requireCredentialsOutlive(t, echoOutput(t, echo, "exchangeExpiration")) +} From 6727128359e972cde43420d0e749dfe78cfe8163 Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Thu, 27 Aug 2026 17:55:22 -0700 Subject: [PATCH 3/7] fix(ci): give the e2e-test AWS profile its session token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `formae connect --profile-aws` failed in CI with InvalidClientTokenId. The profile the workflow writes carries an access key and a secret key but no session token, and the credentials it is built from are temporary ones from an assumed role, which AWS refuses without it. Nothing caught it because nothing had read the profile as a profile. The credentials action also exports the three values as job environment variables, the SDK's default chain prefers those over shared config, and the access-test step passes no --profile — so every other e2e test authenticates through the environment and the half-written profile is never consulted. `--profile-aws` resolves through config.WithSharedConfigProfile, which forces the file and bypasses the environment, making it the first thing here to depend on those three lines being complete. --- .github/workflows/e2e-tests.yml | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index dc77788da..5008bcb52 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -221,10 +221,19 @@ jobs: output-credentials: true - name: Add profile credentials to ~/.aws/credentials + # The session token is not optional: these are temporary credentials + # from an assumed role, and the key pair alone is refused with + # InvalidClientTokenId. It went unnoticed while nothing read the + # profile as a profile — the action also exports the three values as + # job env vars, the SDK's default chain prefers those over shared + # config, and the access test below passes no --profile. `formae + # connect --profile-aws` forces the shared profile, so it is the first + # thing here that authenticates through this file. run: | mkdir -p ~/.aws aws configure set aws_access_key_id ${{ steps.creds.outputs.aws-access-key-id }} --profile e2e-test aws configure set aws_secret_access_key ${{ steps.creds.outputs.aws-secret-access-key }} --profile e2e-test + aws configure set aws_session_token ${{ steps.creds.outputs.aws-session-token }} --profile e2e-test aws configure set region us-west-2 --profile e2e-test - name: Test AWS Access @@ -240,6 +249,18 @@ jobs: tenant-id: ${{ secrets.AZURE_TENANT_ID }} subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} + # Only the oidc-credential entry federates into GCP, and only it pays + # for the sign-in. Gated on the secret as well as the test, so a fork + # without credentials reports the GCP half as skipped rather than red. + - name: Configure GCP Credentials + if: ${{ matrix.test == 'TestOidcCredential' && env.GCP_WORKLOAD_IDENTITY_PROVIDER != '' }} + env: + GCP_WORKLOAD_IDENTITY_PROVIDER: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }} + uses: google-github-actions/auth@v3 + with: + workload_identity_provider: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }} + service_account: ${{ secrets.GCP_SERVICE_ACCOUNT }} + - name: Install system plugins via orbital if: ${{ matrix.system_plugins }} run: | @@ -329,6 +350,13 @@ jobs: env: AWS_PROFILE: e2e-test AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }} + # The project the oidc-credential suite federates into. Its own + # project, not the one the GCP plugin's suites use: provx fixes the + # workload identity pool and provider ids per project and refuses to + # converge a provider trusting a different issuer, so a project + # already carrying the production-issuer connection cannot carry the + # e2e's as well. Absent, the GCP half skips. + E2E_GCP_PROJECT: ${{ secrets.GCP_PROJECT_ID }} # Unique per run+attempt so fixtures can name globally-scoped cloud # resources (e.g. CloudFront KeyValueStore/Function, which aws-nuke # does not clean) without colliding across reruns or concurrent jobs. From 905e931f0ebc5a837154b77584926bad63a94ab1 Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Thu, 27 Aug 2026 17:55:22 -0700 Subject: [PATCH 4/7] fix(connect): provision GCP federation for the issuer the session validated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `connect gcp` read an issuer, held it against the one the control plane reports, and then dropped it: the call to provx built a workload identity provider trusting whatever issuer that library had compiled in. The two agree in production, so the effect was latent there and total everywhere else — provisioning reported success while establishing trust that could never accept a token. provx now takes the issuer as a parameter, as its AWS half always has, so passing the validated one through is the whole fix. The dependency moves with it. This also switches the GCP half of the oidc-credential e2e on. It federates into its own project rather than the one the GCP plugin's suites use: the pool and provider ids are fixed per project and provx refuses to converge a provider trusting a different issuer, so a project already carrying the production-issuer connection cannot carry a test issuer as well. Absent the project, the test skips. --- go.mod | 2 +- go.sum | 2 ++ internal/cli/connect/gcpprovision.go | 8 ++++---- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index f1dd83165..1e32b2183 100644 --- a/go.mod +++ b/go.mod @@ -62,7 +62,7 @@ require ( github.com/platform-engineering-labs/formae/pkg/plugin v0.0.0-00010101000000-000000000000 github.com/platform-engineering-labs/formae/tests/testcontrol v0.0.0-00010101000000-000000000000 github.com/platform-engineering-labs/jsonpatch v0.0.0-20260620044942-701436c7758c - github.com/platform-engineering-labs/oox/provx v0.0.0-20260825164708-d2e52420e91b // re-pin to a real tag at release time + github.com/platform-engineering-labs/oox/provx v0.0.0-20260828004425-f2ab99b17591 // re-pin to a real tag at release time github.com/platform-engineering-labs/orbital v0.2.5 github.com/posthog/posthog-go v1.6.3 github.com/pressly/goose/v3 v3.26.0 diff --git a/go.sum b/go.sum index 7d24cd037..e0c43b738 100644 --- a/go.sum +++ b/go.sum @@ -467,6 +467,8 @@ github.com/platform-engineering-labs/oox/gcpname v0.0.0-20260825170105-3bd97cb18 github.com/platform-engineering-labs/oox/gcpname v0.0.0-20260825170105-3bd97cb18d15/go.mod h1:wytVDQEOgo/0PLmMj0nOnisBzzqwemanNy9gpueH9Ik= github.com/platform-engineering-labs/oox/provx v0.0.0-20260825164708-d2e52420e91b h1:Qh9/9eWqFTK8w/yYHooGVOuRU5ep17Ws7iRVJvthRDg= github.com/platform-engineering-labs/oox/provx v0.0.0-20260825164708-d2e52420e91b/go.mod h1:6dTkhIgTXyXHctVmK16kkM2qnfaX/FOT0aWA+9oeJtM= +github.com/platform-engineering-labs/oox/provx v0.0.0-20260828004425-f2ab99b17591 h1:wr3lRnfa/LRXia1C/VYcqMU4Tkjo4dnSSD+EAiGgRkA= +github.com/platform-engineering-labs/oox/provx v0.0.0-20260828004425-f2ab99b17591/go.mod h1:6dTkhIgTXyXHctVmK16kkM2qnfaX/FOT0aWA+9oeJtM= github.com/platform-engineering-labs/orbital v0.2.5 h1:7rihasWnR68ORytqLubR5WMJchv/mxoWSCB7MqaB9tU= github.com/platform-engineering-labs/orbital v0.2.5/go.mod h1:wwVPqOmW5RO78dDzL/G5CN1Ioao0P/aUsOZVrPVx64s= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= diff --git a/internal/cli/connect/gcpprovision.go b/internal/cli/connect/gcpprovision.go index 9f6ef4fdf..421235c8a 100644 --- a/internal/cli/connect/gcpprovision.go +++ b/internal/cli/connect/gcpprovision.go @@ -20,15 +20,15 @@ type gcpProvisioner interface { } // newGCPProvisioner is the seam tests substitute. Production constructs provx -// with the server-produced subject verbatim: connect has no naming knowledge -// of its own, and inventing a subject here would produce trust the issuer -// never mints for. +// with the server-produced subject and the pinned issuer verbatim: connect has +// no naming knowledge of its own, and inventing either here would produce +// trust the issuer never mints for. var newGCPProvisioner = func(ctx context.Context, project, subject, issuer string) (gcpProvisioner, error) { tenantID, installationID, err := splitSubject(subject) if err != nil { return nil, err } - return provxgcp.New(ctx, slog.Default(), project, tenantID, installationID) + return provxgcp.New(ctx, slog.Default(), project, tenantID, installationID, issuer) } // provisionGCP converges the project's federation and reports what it created. From 9de1346cfee1763e41de14914ce99dd1aa8b4f9b Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Fri, 28 Aug 2026 23:38:24 -0700 Subject: [PATCH 5/7] test(e2e): give each connect run its own identity, and take it back afterwards The oidc-credential suite provisioned its trust under fixed names: one subject, one role, one set of GCP bindings, reused by every run. That made a green result mean less than it appeared to. From the second run onward the exchange succeeded against trust the first run had left standing, so a connect that had quietly stopped provisioning, or that failed to repair state something else had broken, would keep passing. On AWS the same fixed name also meant one run's teardown deleted a role another run might still be using, and that a role surviving a crashed run could be ridden to a pass by a later broken one. The subject and the role name are now derived from a fingerprint of FORMAE_TEST_RUN_ID, which the workflow already sets per run and attempt, falling back to the clock so a developer machine gets the same property. An exchange that succeeds is therefore attributable to the invocation under test: the trust it spends did not exist before that invocation created it. Per-run subjects make the GCP teardown mandatory rather than optional. The provisioner grants its principal roles/editor and roles/resourcemanager.projectIamAdmin, so a fixed subject left one standing grant behind while a per-run one would deposit a new privileged principal on every run, permanently, on a project whose issuer signs with a standing key. The bindings are now revoked in cleanup through the project IAM policy. The pool and provider are deliberately left alone: both are shared between installations, and provx's own Delete leaves them for that reason. The stub control plane also stops accepting requests the real API would reject. It now asserts the installation the run addressed rather than answering for any, refuses unknown fields and trailing content, and reads the trust coordinates through pointers so an absent field is told apart from an empty one. That last one retires a false claim in a comment here: the GCP test said it proved roleArn never crossed the wire, when both an absent field and "" had been decoding to the same empty string. --- tests/e2e/go/connect.go | 126 +++++++++++++-- tests/e2e/go/gcpiam.go | 193 +++++++++++++++++++++++ tests/e2e/go/go.mod | 4 +- tests/e2e/go/go.sum | 4 + tests/e2e/go/oidc_credential_gcp_test.go | 46 +++--- tests/e2e/go/oidc_credential_test.go | 63 +++++--- 6 files changed, 377 insertions(+), 59 deletions(-) create mode 100644 tests/e2e/go/gcpiam.go diff --git a/tests/e2e/go/connect.go b/tests/e2e/go/connect.go index 4f2a0d5ba..77d0d1b62 100644 --- a/tests/e2e/go/connect.go +++ b/tests/e2e/go/connect.go @@ -9,8 +9,11 @@ package e2e_test import ( "bytes" "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" + "io" "net/http" "net/http/httptest" "os" @@ -43,10 +46,40 @@ import ( // the auth plugin is refused rather than served. const oidcAuthStubBearer = "Bearer e2e-oidc-connect-token" -// connectInstallationID is the installation a connect run addresses. The -// profile loader requires 27 base62 characters. Nothing resolves it: the stub -// answers for whichever installation the run names. -const connectInstallationID = "2eOidcConnectE2eInstall0001" +// testRunIDEnv carries a value unique to one workflow run and attempt. The +// e2e workflow sets it; a developer run has none. +const testRunIDEnv = "FORMAE_TEST_RUN_ID" + +// runFingerprint is this run's identity, computed once per test process. +// +// Everything connect provisions is named from it, which is what makes a green +// run mean something. With a fixed identity, the second run onwards exchanges +// against trust the first one left behind: a connect that had silently stopped +// provisioning would keep passing, and so would one that failed to repair +// state some other actor had broken. Naming the identity per run makes the +// exchange attributable to the invocation under test. +// +// It falls back to the clock rather than failing, so the suite still runs on a +// developer machine — where two runs sharing an identity is the same hazard, +// one account down. +var runFingerprint = sync.OnceValue(func() string { + seed := os.Getenv(testRunIDEnv) + if seed == "" { + seed = fmt.Sprintf("local-%d", time.Now().UnixNano()) + } + sum := sha256.Sum256([]byte(seed)) + return hex.EncodeToString(sum[:]) +}) + +// ConnectInstallationID is the installation a connect run addresses. The +// profile loader requires 27 base62 characters, and hex satisfies that. +// Nothing resolves it: the stub answers for whichever installation the run +// names, and it is per-run so the subject built from it is too. +func ConnectInstallationID() string { return runFingerprint()[:27] } + +// ConnectRunSuffix is the short discriminator that per-run cloud resource +// names carry. +func ConnectRunSuffix() string { return runFingerprint()[:12] } // oidcAuthPluginDirEnv names the plugin tree holding the stub auth plugin, // staged by `make test-e2e`. @@ -61,14 +94,43 @@ type connectSetup struct { Issuer string `json:"issuer"` } -// stubRegistration is one registration the stub control plane received. The -// coordinate fields mirror the CLI's own registration body, so a test can -// assert on what actually crossed the wire. +// stubRegistration is one registration the stub control plane received. +// +// The coordinate fields are pointers because the control plane's real schema +// is a discriminated union that admits exactly one shape per cloud and rejects +// a field belonging to another. Absent and present-but-empty are therefore +// different answers, and a plain string cannot tell them apart — which would +// let a test claim the wrong coordinate never crossed the wire when in fact it +// crossed as "". type stubRegistration struct { - Cloud string `json:"cloud"` - Account string `json:"account"` - RoleArn string `json:"roleArn"` - WorkloadIdentityProvider string `json:"workloadIdentityProvider"` + Cloud string `json:"cloud"` + Account string `json:"account"` + RoleArn *string `json:"roleArn"` + WorkloadIdentityProvider *string `json:"workloadIdentityProvider"` +} + +// Coordinate returns the trust coordinate this registration carries, and +// whether the field was present at all. +func (r stubRegistration) Coordinate() (string, bool) { + if r.Cloud == "gcp" { + if r.WorkloadIdentityProvider == nil { + return "", false + } + return *r.WorkloadIdentityProvider, true + } + if r.RoleArn == nil { + return "", false + } + return *r.RoleArn, true +} + +// ForeignCoordinatePresent reports whether the registration carried the field +// belonging to a different cloud, which the real schema would reject. +func (r stubRegistration) ForeignCoordinatePresent() bool { + if r.Cloud == "gcp" { + return r.RoleArn != nil + } + return r.WorkloadIdentityProvider != nil } // connectStub is a running stub control plane. @@ -99,7 +161,7 @@ func StartConnectStub(t *testing.T, setup connectSetup) *connectStub { mux.HandleFunc("GET /api/v1/installations/{installation}/cloud-connection-setup", func(w http.ResponseWriter, r *http.Request) { - if !stubAuthorized(t, w, r) { + if !stubAuthorized(t, w, r) || !stubAddressedCorrectly(t, w, r) { return } writeStubJSON(t, w, http.StatusOK, setup) @@ -107,12 +169,12 @@ func StartConnectStub(t *testing.T, setup connectSetup) *connectStub { mux.HandleFunc("POST /api/v1/installations/{installation}/cloud-connections", func(w http.ResponseWriter, r *http.Request) { - if !stubAuthorized(t, w, r) { + if !stubAuthorized(t, w, r) || !stubAddressedCorrectly(t, w, r) { return } var registration stubRegistration - if err := json.NewDecoder(r.Body).Decode(®istration); err != nil { - t.Errorf("stub control plane: registration body is not JSON: %v", err) + if err := decodeStrict(r.Body, ®istration); err != nil { + t.Errorf("stub control plane: registration body: %v", err) w.WriteHeader(http.StatusBadRequest) return } @@ -145,6 +207,38 @@ func stubAuthorized(t *testing.T, w http.ResponseWriter, r *http.Request) bool { return true } +// stubAddressedCorrectly holds the run to the installation it configured. The +// route variable is the only place the profile's installation id reaches the +// control plane, so serving any id would let a run that addressed the wrong +// installation pass. +func stubAddressedCorrectly(t *testing.T, w http.ResponseWriter, r *http.Request) bool { + t.Helper() + + if got := r.PathValue("installation"); got != ConnectInstallationID() { + t.Errorf("stub control plane: request addressed installation %q, want %q", got, ConnectInstallationID()) + writeStubJSON(t, w, http.StatusNotFound, map[string]any{ + "error": map[string]any{"code": "not_found"}, + }) + return false + } + return true +} + +// decodeStrict reads exactly one JSON document into v and refuses anything +// the real control plane would: an unknown field, or trailing content after +// the object. +func decodeStrict(r io.Reader, v any) error { + dec := json.NewDecoder(r) + dec.DisallowUnknownFields() + if err := dec.Decode(v); err != nil { + return fmt.Errorf("not a JSON object of the expected shape: %w", err) + } + if err := dec.Decode(new(json.RawMessage)); err != io.EOF { + return fmt.Errorf("carries content after the JSON object") + } + return nil +} + func writeStubJSON(t *testing.T, w http.ResponseWriter, status int, body any) { t.Helper() @@ -193,7 +287,7 @@ func WriteHostedConnectConfig(t *testing.T, dir, authPluginDir, issuer string) s t.Helper() path := filepath.Join(dir, "connect-config.pkl") - content := fmt.Sprintf(hostedConnectConfig, authPluginDir, connectInstallationID, issuer) + content := fmt.Sprintf(hostedConnectConfig, authPluginDir, ConnectInstallationID(), issuer) if err := os.WriteFile(path, []byte(content), 0o644); err != nil { t.Fatalf("failed to write the connect config: %v", err) } diff --git a/tests/e2e/go/gcpiam.go b/tests/e2e/go/gcpiam.go new file mode 100644 index 000000000..e57270570 --- /dev/null +++ b/tests/e2e/go/gcpiam.go @@ -0,0 +1,193 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +//go:build e2e + +package e2e_test + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "testing" + "time" + + "golang.org/x/oauth2/google" +) + +// Revoking what `formae connect gcp` granted. +// +// The provisioner deliberately leaves the pool and the provider standing — +// both are shared, and deleting them would revoke every other installation +// connected to the same project — and removes only the IAM bindings for its +// own principal. This does the same, because the alternative is worse than +// leaving one set behind: the subject is per-run, so without this every run +// would deposit another principal holding roles/editor and +// roles/resourcemanager.projectIamAdmin, permanently, on a project whose +// issuer's signing key is a standing fixture. + +const ( + // gcpIamPolicyURL is the project-level IAM policy endpoint. v1 is what + // provx uses for the same bindings. + gcpIamPolicyURL = "https://cloudresourcemanager.googleapis.com/v1/projects/" + + // gcpPolicyVersion asks for the representation that can express + // conditional bindings. The bindings here are unconditional, but a policy + // read at a lower version silently drops any conditional binding it cannot + // represent, and writing that back would delete somebody else's grant. + gcpPolicyVersion = 3 +) + +// gcpPolicy is the project IAM policy, read and written whole. Unknown fields +// are preserved only insofar as they are not needed: the etag makes the +// read-modify-write safe, so a concurrent editor loses the race rather than +// having their change silently dropped. +type gcpPolicy struct { + Version int `json:"version"` + Etag string `json:"etag"` + Bindings []gcpBinding `json:"bindings"` +} + +type gcpBinding struct { + Role string `json:"role"` + Members []string `json:"members"` + Condition json.RawMessage `json:"condition,omitempty"` +} + +// RevokeGCPPrincipal removes every binding member equal to principal from the +// project's IAM policy. +// +// A failure here does fail the run, deliberately: bindings this run granted and +// could not take back are exactly the thing worth being told about, and a +// cleanup that reported nothing would leave them to accumulate unnoticed. It +// records rather than aborts, so one failed step does not skip the rest. +func RevokeGCPPrincipal(t *testing.T, project, principal string) { + t.Helper() + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + + client, err := google.DefaultClient(ctx, "https://www.googleapis.com/auth/cloud-platform") + if err != nil { + t.Errorf("cleanup: no Google credentials to revoke %s with: %v", principal, err) + return + } + + // One retry: the read-modify-write races anything else editing the policy, + // and the etag turns that race into a 409 rather than a lost update. + for attempt := range 2 { + policy, err := getGCPPolicy(ctx, client, project) + if err != nil { + t.Errorf("cleanup: reading the IAM policy of %s: %v", project, err) + return + } + + filtered, removed := withoutMember(policy.Bindings, principal) + if !removed { + return // nothing bound; a run that failed before provisioning + } + policy.Bindings = filtered + policy.Version = gcpPolicyVersion + + err = setGCPPolicy(ctx, client, project, policy) + if err == nil { + return + } + if attempt == 1 || !strings.Contains(err.Error(), "HTTP 409") { + t.Errorf("cleanup: revoking %s on %s: %v", principal, project, err) + return + } + } +} + +// withoutMember drops member from every binding, and drops any binding it +// empties: a binding with no members is not valid to write back. +func withoutMember(bindings []gcpBinding, member string) ([]gcpBinding, bool) { + var kept []gcpBinding + removed := false + for _, b := range bindings { + var members []string + for _, m := range b.Members { + if m == member { + removed = true + continue + } + members = append(members, m) + } + if len(members) == 0 { + continue + } + b.Members = members + kept = append(kept, b) + } + return kept, removed +} + +func getGCPPolicy(ctx context.Context, client *http.Client, project string) (*gcpPolicy, error) { + body, _ := json.Marshal(map[string]any{ + "options": map[string]any{"requestedPolicyVersion": gcpPolicyVersion}, + }) + data, err := gcpIamCall(ctx, client, gcpIamPolicyURL+project+":getIamPolicy", body) + if err != nil { + return nil, err + } + var policy gcpPolicy + if err := json.Unmarshal(data, &policy); err != nil { + return nil, fmt.Errorf("parsing the IAM policy: %w", err) + } + return &policy, nil +} + +func setGCPPolicy(ctx context.Context, client *http.Client, project string, policy *gcpPolicy) error { + body, err := json.Marshal(map[string]any{"policy": policy}) + if err != nil { + return err + } + _, err = gcpIamCall(ctx, client, gcpIamPolicyURL+project+":setIamPolicy", body) + return err +} + +func gcpIamCall(ctx context.Context, client *http.Client, url string, body []byte) ([]byte, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(body)) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/json") + + resp, err := client.Do(req) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + + data, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return nil, err + } + if resp.StatusCode < 200 || resp.StatusCode > 299 { + return nil, fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(data)) + } + return data, nil +} + +// GCPPrincipalFor builds the IAM member string for a federated subject in the +// pool the given provider belongs to. +// +// It is derived from the provider resource name connect reported rather than +// assembled from parts the test guesses at, so a cleanup can only ever revoke +// the principal that this run's own registration named. +func GCPPrincipalFor(t *testing.T, providerName, subject string) string { + t.Helper() + + // //iam.googleapis.com/projects/N/locations/global/workloadIdentityPools/P/providers/Q + pool, _, found := strings.Cut(providerName, "/providers/") + if !found { + t.Fatalf("workload identity provider %q does not name a pool", providerName) + } + return "principal:" + pool + "/subject/" + subject +} diff --git a/tests/e2e/go/go.mod b/tests/e2e/go/go.mod index 2a1e8503c..e6bbf2fc2 100644 --- a/tests/e2e/go/go.mod +++ b/tests/e2e/go/go.mod @@ -1,6 +1,6 @@ module github.com/platform-engineering-labs/formae/tests/e2e -go 1.25 +go 1.25.0 require ( github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1 @@ -10,9 +10,11 @@ require ( github.com/aws/aws-sdk-go-v2/config v1.32.10 github.com/aws/aws-sdk-go-v2/service/iam v1.53.3 golang.org/x/crypto v0.41.0 + golang.org/x/oauth2 v0.36.0 ) require ( + cloud.google.com/go/compute/metadata v0.3.0 // indirect github.com/Azure/azure-sdk-for-go/sdk/azcore v1.20.0 // indirect github.com/Azure/azure-sdk-for-go/sdk/internal v1.11.2 // indirect github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 // indirect diff --git a/tests/e2e/go/go.sum b/tests/e2e/go/go.sum index 479f2530d..b3088bb92 100644 --- a/tests/e2e/go/go.sum +++ b/tests/e2e/go/go.sum @@ -1,3 +1,5 @@ +cloud.google.com/go/compute/metadata v0.3.0 h1:Tz+eQXMEqDIKRsmY3cHTL6FVaynIjX2QxYC4trgAKZc= +cloud.google.com/go/compute/metadata v0.3.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k= github.com/Azure/azure-sdk-for-go/sdk/azcore v1.20.0 h1:JXg2dwJUmPB9JmtVmdEB16APJ7jurfbY5jnfXpJoRMc= github.com/Azure/azure-sdk-for-go/sdk/azcore v1.20.0/go.mod h1:YD5h/ldMsG0XiIw7PdyNhLxaM317eFh5yNLccNfGdyw= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1 h1:Hk5QBxZQC1jb2Fwj6mpzme37xbCDdNTxU7O9eb5+LB4= @@ -77,6 +79,8 @@ golang.org/x/crypto v0.41.0/go.mod h1:pO5AFd7FA68rFak7rOAGVuygIISepHftHnr8dr6+sU golang.org/x/mod v0.26.0/go.mod h1:/j6NAhSk8iQ723BGAUyoAcn7SlD7s15Dp9Nd/SfeaFQ= golang.org/x/net v0.43.0 h1:lat02VYK2j4aLzMzecihNvTlJNQUq316m2Mr9rnM6YE= golang.org/x/net v0.43.0/go.mod h1:vhO1fvI4dGsIjh73sWfUVjj3N7CA9WkKJNQm2svM6Jg= +golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= +golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.35.0 h1:vz1N37gP5bs89s7He8XuIYXpyY0+QlsKmzipCbUtyxI= diff --git a/tests/e2e/go/oidc_credential_gcp_test.go b/tests/e2e/go/oidc_credential_gcp_test.go index c543c7307..cab7c9a45 100644 --- a/tests/e2e/go/oidc_credential_gcp_test.go +++ b/tests/e2e/go/oidc_credential_gcp_test.go @@ -41,20 +41,21 @@ func gcpE2EProject(t *testing.T) string { // connectProvisionsGcpTrust drives `formae connect gcp` against a stub control // plane and returns the workload identity provider it provisioned. // -// Nothing is torn down afterwards, deliberately. Everything connect creates -// here converges rather than accumulates: the pool and the provider are fixed -// per project and are patched in place, and the IAM bindings are set for one -// principal that does not change between runs. provx's own Delete leaves the -// pool and provider standing for the same reason — they are shared — so a -// teardown here would remove two bindings the next run puts straight back. +// The pool and the provider are left standing: both are fixed per project and +// shared between installations, and provx's own Delete leaves them for that +// reason. The IAM bindings are not left standing. The subject is this run's, +// so they would accumulate one privileged principal per run, each holding +// roles/editor and roles/resourcemanager.projectIamAdmin on a project whose +// issuer signs with a standing key — a growing grant with no expiry and no +// owner. func connectProvisionsGcpTrust(t *testing.T, bin, project string) string { t.Helper() stub := StartConnectStub(t, connectSetup{ - CloudSubject: oidcEchoSubject, + CloudSubject: oidcEchoSubject(), // GCP carries no role, but the setup read requires all three // coordinates: the control plane serves one document for every cloud. - CloudRoleName: oidcConnectRoleName, + CloudRoleName: oidcConnectRoleName(), Issuer: oidcEchoIssuer, }) configPath := WriteHostedConnectConfig(t, t.TempDir(), @@ -83,20 +84,29 @@ func connectProvisionsGcpTrust(t *testing.T, bin, project string) string { t.Fatalf("connect workloadIdentityProvider %q is not a provider resource name", doc.WorkloadIdentityProvider) } + // Registered only once the provider name is known, since the principal is + // derived from it. A run that fails between here and the end still revokes + // what it granted. + t.Cleanup(func() { + RevokeGCPPrincipal(t, project, GCPPrincipalFor(t, doc.WorkloadIdentityProvider, oidcEchoSubject())) + }) + registrations := stub.Registrations() if len(registrations) != 1 { t.Fatalf("stub control plane received %d registrations, want 1: %+v", len(registrations), registrations) } got := registrations[0] - if got.Cloud != "gcp" || got.Account != project || got.WorkloadIdentityProvider != doc.WorkloadIdentityProvider { - t.Errorf("registration: got %+v, want cloud gcp, account %s, workloadIdentityProvider %s", - got, project, doc.WorkloadIdentityProvider) + coordinate, present := got.Coordinate() + if got.Cloud != "gcp" || got.Account != project || !present || coordinate != doc.WorkloadIdentityProvider { + t.Errorf("registration: got cloud %q account %q workloadIdentityProvider %q (present %v), want cloud gcp, account %s, provider %s", + got.Cloud, got.Account, coordinate, present, project, doc.WorkloadIdentityProvider) } - // The AWS coordinate is not merely empty on the wire, it is absent: the - // control plane's schema is a discriminated union that rejects a field - // belonging to another variant. - if got.RoleArn != "" { - t.Errorf("registration carried a roleArn %q on a GCP connection", got.RoleArn) + // Absent, not merely empty: the control plane's schema is a discriminated + // union that rejects a field belonging to another variant, so a roleArn + // sent as "" would be refused just as one carrying a value would. The + // decoder keeps the two apart rather than collapsing both to "". + if got.ForeignCoordinatePresent() { + t.Errorf("registration carried a roleArn field on a GCP connection") } return doc.WorkloadIdentityProvider @@ -122,7 +132,7 @@ func TestOidcCredential_GcpTokenExchangesForRealCredentials(t *testing.T) { agent := StartAgent(t, bin, WithPluginDir(stagedOidcPluginDir(t, oidcPluginDirEnv)), WithEnv( - "E2E_OIDC_SUBJECT="+oidcEchoSubject, + "E2E_OIDC_SUBJECT="+oidcEchoSubject(), // On GCP the audience is the provider's own resource name, and // Google pins the provider's allowed audiences to that same // string, so the token and the exchange cannot disagree. @@ -147,7 +157,7 @@ func TestOidcCredential_GcpTokenExchangesForRealCredentials(t *testing.T) { {header, "alg", "RS256"}, {header, "kid", oidcEchoKeyID}, {claims, "iss", oidcEchoIssuer}, - {claims, "sub", oidcEchoSubject}, + {claims, "sub", oidcEchoSubject()}, {claims, "aud", provider}, } { if got := jwtString(t, check.doc, check.key); got != check.expected { diff --git a/tests/e2e/go/oidc_credential_test.go b/tests/e2e/go/oidc_credential_test.go index 8662b0fb3..c99ce32bf 100644 --- a/tests/e2e/go/oidc_credential_test.go +++ b/tests/e2e/go/oidc_credential_test.go @@ -27,25 +27,15 @@ const ( oidcEchoIssuer = "https://e2e-oidc-issuer-942849037363.s3.us-west-2.amazonaws.com" oidcEchoKeyID = "e2e-oidc-key-1" - // The subject the control plane produces, in the grammar it really uses: - // the `fai:` namespace, a tenant, and the installation being connected. - // Connect takes it verbatim into the trust it provisions and the GCP path - // parses it back apart, so a fixture subject of a made-up shape would - // exercise a string production never emits. + // The tenant half of the subject. The installation half is per-run, which + // is what makes the trust connect provisions per-run too. oidcEchoTenantID = "2eOidcConnectE2eTenant00001" - oidcEchoSubject = "fai:" + oidcEchoTenantID + "/" + connectInstallationID // The e2e account, pinned rather than derived: connect is told which // account to connect and never infers one from ambient credentials, and // the test asserts on the role ARN that produces. oidcEchoAccount = "942849037363" - // The role connect provisions and the echo plugin then assumes. The name - // carries the suite's own prefix so the pre-cleanup purge reclaims it if - // a run dies before its teardown; a survivor is converged rather than - // refused, since it carries the ownership tags connect wrote. - oidcConnectRoleName = "formae-e2e-oidc-connect-role" - // The AWS shared-config profile connect provisions with, written by the // e2e workflow's credentials step. oidcConnectAWSProfile = "e2e-test" @@ -63,6 +53,26 @@ const ( oidcPluginDirNoBrokerEnv = "E2E_OIDC_PLUGIN_DIR_NO_BROKER" ) +// oidcEchoSubject is the subject the control plane produces, in the grammar it +// really uses: the `fai:` namespace, a tenant, and the installation being +// connected. Connect takes it verbatim into the trust it provisions and the +// GCP path parses it back apart, so a subject of a made-up shape would +// exercise a string production never emits. +// +// The installation half is this run's, so the trust provisioned against it is +// this run's: an exchange that succeeds could not have been riding on what an +// earlier run established. +func oidcEchoSubject() string { return "fai:" + oidcEchoTenantID + "/" + ConnectInstallationID() } + +// oidcConnectRoleName is the role connect provisions and the echo plugin then +// assumes. +// +// Per-run for the same reason as the subject, and because cleanup deletes this +// role outright: a shared name would let one run's teardown remove trust +// another run was still using. The suite's own prefix keeps it inside the +// pre-cleanup purge, so a run that dies before its teardown is reclaimed. +func oidcConnectRoleName() string { return "formae-e2e-oidc-connect-" + ConnectRunSuffix() } + // stagedOidcPluginDir returns the staged plugin tree named by the given // environment variable. `make test-e2e` builds both fixture binaries and // stages them; running the suite by hand without it leaves nothing to @@ -189,8 +199,8 @@ func connectProvisionsTrust(t *testing.T, bin string) string { t.Helper() stub := StartConnectStub(t, connectSetup{ - CloudSubject: oidcEchoSubject, - CloudRoleName: oidcConnectRoleName, + CloudSubject: oidcEchoSubject(), + CloudRoleName: oidcConnectRoleName(), Issuer: oidcEchoIssuer, }) configPath := WriteHostedConnectConfig(t, t.TempDir(), @@ -198,7 +208,7 @@ func connectProvisionsTrust(t *testing.T, bin string) string { // Registered before the run, not after it: a run that provisions the role // and then fails to register still leaves the role standing. - t.Cleanup(func() { DeleteIAMRole(t, oidcConnectRoleName) }) + t.Cleanup(func() { DeleteIAMRole(t, oidcConnectRoleName()) }) doc := RunConnect(t, bin, ConnectEnv(stub.URL, oidcEchoIssuer), "aws", @@ -217,8 +227,8 @@ func connectProvisionsTrust(t *testing.T, bin string) string { if doc.Account != oidcEchoAccount { t.Errorf("connect account: got %q, want %q", doc.Account, oidcEchoAccount) } - if !strings.HasSuffix(doc.RoleArn, ":role/"+oidcConnectRoleName) { - t.Fatalf("connect roleArn %q does not name role %q", doc.RoleArn, oidcConnectRoleName) + if !strings.HasSuffix(doc.RoleArn, ":role/"+oidcConnectRoleName()) { + t.Fatalf("connect roleArn %q does not name role %q", doc.RoleArn, oidcConnectRoleName()) } // What the control plane was told, rather than what the CLI printed: the @@ -228,9 +238,14 @@ func connectProvisionsTrust(t *testing.T, bin string) string { if len(registrations) != 1 { t.Fatalf("stub control plane received %d registrations, want 1: %+v", len(registrations), registrations) } - if got := registrations[0]; got.Cloud != "aws" || got.Account != oidcEchoAccount || got.RoleArn != doc.RoleArn { - t.Errorf("registration: got %+v, want cloud aws, account %s, roleArn %s", - got, oidcEchoAccount, doc.RoleArn) + got := registrations[0] + coordinate, present := got.Coordinate() + if got.Cloud != "aws" || got.Account != oidcEchoAccount || !present || coordinate != doc.RoleArn { + t.Errorf("registration: got cloud %q account %q roleArn %q (present %v), want cloud aws, account %s, roleArn %s", + got.Cloud, got.Account, coordinate, present, oidcEchoAccount, doc.RoleArn) + } + if got.ForeignCoordinatePresent() { + t.Errorf("registration carried a GCP coordinate on an AWS connection") } return doc.RoleArn @@ -251,7 +266,7 @@ func TestOidcCredential_TokenExchangesForRealCredentials(t *testing.T) { agent := StartAgent(t, bin, WithPluginDir(stagedOidcPluginDir(t, oidcPluginDirEnv)), WithEnv( - "E2E_OIDC_SUBJECT="+oidcEchoSubject, + "E2E_OIDC_SUBJECT="+oidcEchoSubject(), "E2E_OIDC_AUDIENCE="+oidcEchoAudience, "E2E_OIDC_ASSUME_ROLE_ARN="+roleArn, ), @@ -273,7 +288,7 @@ func TestOidcCredential_TokenExchangesForRealCredentials(t *testing.T) { {header, "alg", "RS256"}, {header, "kid", oidcEchoKeyID}, {claims, "iss", oidcEchoIssuer}, - {claims, "sub", oidcEchoSubject}, + {claims, "sub", oidcEchoSubject()}, {claims, "aud", oidcEchoAudience}, } { if got := jwtString(t, check.doc, check.key); got != check.expected { @@ -284,8 +299,8 @@ func TestOidcCredential_TokenExchangesForRealCredentials(t *testing.T) { if got := echoOutput(t, echo, "exchangeError"); got != "" { t.Fatalf("exchangeError: got %q, want empty", got) } - if got := echoOutput(t, echo, "exchangeIdentity"); !strings.Contains(got, oidcConnectRoleName) { - t.Errorf("exchangeIdentity %q does not name role %q", got, oidcConnectRoleName) + if got := echoOutput(t, echo, "exchangeIdentity"); !strings.Contains(got, oidcConnectRoleName()) { + t.Errorf("exchangeIdentity %q does not name role %q", got, oidcConnectRoleName()) } requireCredentialsOutlive(t, echoOutput(t, echo, "exchangeExpiration")) } From 8716a4abe75dc5c083d8809925b7794b393cbb05 Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Fri, 28 Aug 2026 23:38:24 -0700 Subject: [PATCH 6/7] test(e2e): retry the assume only while IAM is propagating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exchange retried every failure for a fixed window measured between attempts. Both halves were wrong in the same direction: they spent time that bought nothing and buried the reason. A wall-clock check between attempts bounds when the last request may start, not when it may finish, so an attempt begun just inside the window could run past the agent's plugin call deadline. The failure then arrives as an operation timeout instead of the recorded exchange error, which reads as the harness breaking rather than as the exchange refusing. The window is now a child context, so nothing the loop starts can outlive it. Retrying every error also meant a settled answer — a malformed ARN, a rejected token, an expired credential — cost the whole window before being reported. Only AccessDenied is waited out now, because that is what STS returns for a role whose trust policy has not propagated yet. It is also what a genuinely wrong trust policy returns, and the two are not distinguishable from outside, which is why this is a bounded wait rather than a poll for readiness. --- tests/e2e/go/fixtures/oidc-echo-plugin/go.mod | 2 +- .../go/fixtures/oidc-echo-plugin/plugin.go | 41 +++++++++++++++---- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/tests/e2e/go/fixtures/oidc-echo-plugin/go.mod b/tests/e2e/go/fixtures/oidc-echo-plugin/go.mod index 5995d2103..b5570813c 100644 --- a/tests/e2e/go/fixtures/oidc-echo-plugin/go.mod +++ b/tests/e2e/go/fixtures/oidc-echo-plugin/go.mod @@ -20,6 +20,7 @@ require ( github.com/aws/aws-sdk-go-v2 v1.43.7 github.com/aws/aws-sdk-go-v2/config v1.32.38 github.com/aws/aws-sdk-go-v2/service/sts v1.45.7 + github.com/aws/smithy-go v1.27.8 github.com/platform-engineering-labs/formae/pkg/model v0.1.6 github.com/platform-engineering-labs/formae/pkg/plugin v0.0.0 ) @@ -38,7 +39,6 @@ require ( github.com/aws/aws-sdk-go-v2/service/signin v1.5.7 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.33.7 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.38.7 // indirect - github.com/aws/smithy-go v1.27.8 // indirect github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/ebitengine/purego v0.9.1 // indirect diff --git a/tests/e2e/go/fixtures/oidc-echo-plugin/plugin.go b/tests/e2e/go/fixtures/oidc-echo-plugin/plugin.go index 3a2850f1e..fa4faa061 100644 --- a/tests/e2e/go/fixtures/oidc-echo-plugin/plugin.go +++ b/tests/e2e/go/fixtures/oidc-echo-plugin/plugin.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -17,6 +18,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" awsconfig "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/service/sts" + "github.com/aws/smithy-go" pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" "github.com/platform-engineering-labs/formae/pkg/plugin" "github.com/platform-engineering-labs/formae/pkg/plugin/resource" @@ -186,12 +188,15 @@ type awsExchange struct { // ordinary state of a role `formae connect` made seconds earlier, so reporting // the first refusal would fail the run over a trust policy that is correct. // -// The window is well inside the agent's 60s plugin call deadline, so this -// gives up before the operator does and reports STS's own last refusal. A -// genuinely wrong trust policy still fails, at the end of the window rather -// than at the start of it. +// The window is a child deadline, not a wall-clock check between attempts. A +// check between attempts bounds when the last request may *start*, not when it +// may finish, so a slow attempt begun just inside the window can run past the +// agent's 60s plugin call deadline and turn a recorded exchangeError into an +// operation timeout — which reads as the harness breaking rather than as the +// exchange refusing. The margin below leaves the operator's deadline room to +// be the one that never fires. const ( - assumeRolePropagationWindow = 40 * time.Second + assumeRolePropagationWindow = 30 * time.Second assumeRolePropagationInterval = 2 * time.Second ) @@ -202,7 +207,11 @@ func (e awsExchange) Spend(ctx context.Context, token string) (identity, expirat } client := sts.NewFromConfig(cfg) - deadline := time.Now().Add(assumeRolePropagationWindow) + // Bounding the requests themselves, so nothing this loop starts can outlive + // the window. + ctx, cancel := context.WithTimeout(ctx, assumeRolePropagationWindow) + defer cancel() + for { out, err := client.AssumeRoleWithWebIdentity(ctx, &sts.AssumeRoleWithWebIdentityInput{ RoleArn: aws.String(e.roleARN), @@ -218,17 +227,33 @@ func (e awsExchange) Spend(ctx context.Context, token string) (identity, expirat } return identity, expiration, nil } - if time.Now().After(deadline) { + // Only the refusal propagation produces is worth waiting out. A + // malformed ARN, a rejected token or an expired credential is settled + // on the first answer, and spending the whole window on it buries the + // reason under a delay that looks like a hang. + if !awaitingPropagation(err) { return "", "", fmt.Errorf("assume role with web identity: %w", err) } select { case <-ctx.Done(): - return "", "", fmt.Errorf("assume role with web identity: %w", ctx.Err()) + return "", "", fmt.Errorf("assume role with web identity: %w", err) case <-time.After(assumeRolePropagationInterval): } } } +// awaitingPropagation reports whether err is the refusal STS gives for a role +// whose trust policy has not propagated yet. +// +// AccessDenied is also what a genuinely wrong trust policy produces, and the +// two are not distinguishable from the outside — which is the whole reason the +// window exists rather than a poll for readiness. Everything else is a settled +// answer and is returned as it stands. +func awaitingPropagation(err error) bool { + var api smithy.APIError + return errors.As(err, &api) && api.ErrorCode() == "AccessDenied" +} + // probeLabelOf reads the probeLabel property out of a properties document. func probeLabelOf(properties json.RawMessage) (string, error) { var props struct { From 1fd6b31484cb8c1a7b8a5f8c510db295bb9f7501 Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Sat, 29 Aug 2026 00:01:26 -0700 Subject: [PATCH 7/7] test(e2e): fail the GCP cleanup when it revokes nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The revocation treated an empty result as success, which made a principal string that does not match what Google stores indistinguishable from a clean teardown. Both paths returned without complaint, so privileged bindings could survive under a green run — the precise failure the cleanup exists to prevent, reintroduced inside it. It runs only once connect has reported the provider it provisioned, so by then the binding it granted must exist. Finding none is therefore a defect worth reporting, not a state worth tolerating. --- tests/e2e/go/gcpiam.go | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tests/e2e/go/gcpiam.go b/tests/e2e/go/gcpiam.go index e57270570..42b43abcf 100644 --- a/tests/e2e/go/gcpiam.go +++ b/tests/e2e/go/gcpiam.go @@ -60,12 +60,19 @@ type gcpBinding struct { } // RevokeGCPPrincipal removes every binding member equal to principal from the -// project's IAM policy. +// project's IAM policy, and fails the run if it removed nothing. // -// A failure here does fail the run, deliberately: bindings this run granted and -// could not take back are exactly the thing worth being told about, and a -// cleanup that reported nothing would leave them to accumulate unnoticed. It -// records rather than aborts, so one failed step does not skip the rest. +// The empty case is a failure rather than a no-op because of when this is +// called: only once connect has reported the provider it provisioned, by which +// point the binding it granted must exist. Treating "found nothing" as success +// would make a principal string that does not match what Google stores +// indistinguishable from a clean revocation — leaving privileged bindings +// standing under a green run, which is the exact failure this cleanup exists +// to prevent. +// +// A failure does fail the run, deliberately: bindings this run granted and +// could not take back are worth being told about. It records rather than +// aborts, so one failed step does not skip the rest. func RevokeGCPPrincipal(t *testing.T, project, principal string) { t.Helper() @@ -89,7 +96,10 @@ func RevokeGCPPrincipal(t *testing.T, project, principal string) { filtered, removed := withoutMember(policy.Bindings, principal) if !removed { - return // nothing bound; a run that failed before provisioning + t.Errorf("cleanup: %s held no bindings on %s to revoke; connect granted them, so either "+ + "something else removed them or this principal does not name what Google stored", + principal, project) + return } policy.Bindings = filtered policy.Version = gcpPolicyVersion