fix(iam): four phase-3 follow-ups (provider scoping, public path wrapper, static mirror, claim-mode RoleArn) (#9333)

* fix(iam): scope IAM-managed OIDC provider lookup by role account

Two account-scoped OIDC records sharing an issuer were collapsed into a
single map slot keyed only by the URL. The last-write-wins entry then
served every AssumeRoleWithWebIdentity, so a token destined for account
B's role could be validated by account A's record (its clientIDs and
thumbprints), defeating the per-account isolation the records exist for.

The role-account check in enforceProviderAccountScope still rejected
the cross-account assumption, but only after the wrong record's
audience and TLS pin had already accepted the token.

Refresh now keys IAM-managed records as (issuer, account), and
validation parses the requested role's account up front and matches
the record under that issuer in this order: exact account, global
(account-less), static-config fallback. An unknown account hint
deliberately skips account-scoped entries — picking one arbitrarily is
the bug this commit fixes — and falls through to global or static.

* fix(iam): route public AssumeRoleWithWebIdentity through IAMManager

handleAssumeRoleWithWebIdentity called stsService.AssumeRoleWithWebIdentity
directly, bypassing the IAMManager wrapper. The wrapper is where
enforceProviderAccountScope rejects cross-account assumption attempts
and capDurationByRole clamps to the role's MaxSessionDuration; both
silently became no-ops for any AWS-SDK caller hitting the public
endpoint.

Dispatch through the IAMManager (via the existing IAMManagerProvider
interface that other handlers in this file already use) when one is
wired. Embedded test setups without an IAM integration fall back to
the bare STS service unchanged.

* fix(iam): mirror thumbprints, principal-tag keys, and policy claim from static OIDC config

initOIDCProviderStore mirrored only URL and ClientIDs. Once
RefreshOIDCProvidersFromStore ran (on any IAM-managed mutation, or on
boot once the metadata-subscribe loop kicked in),
buildOIDCProviderFromRecord rebuilt the runtime provider from this
truncated record. Because IAM-managed entries take precedence over the
static-config map, the rebuild silently shadowed the bootstrap with a
weaker provider:

- Thumbprints: dropped, so TLS-pinned issuers fell back to the system
  trust store.
- AllowedPrincipalTagKeys: dropped, so principal-tag claims stopped
  reaching the session.
- PolicyClaim: dropped, so claim-based policy mode stopped triggering.

Pull all three from the provider's static Config map at mirror time so
the stored record round-trips to a runtime provider equivalent to the
one the static config produced directly.

* fix(iam): allow empty RoleArn in AssumeRoleWithWebIdentity HTTP handler

Phase 3b advertises that RoleArn MAY be omitted in claim-based policy
mode — the STS service then derives the assumed-role ARN from the
configured policy claim. The HTTP handler still rejected empty RoleArn
up front with MissingParameter, so SDK callers using the documented
omitted-role flow never reached the STS layer.

Drop the pre-check; STS still validates that claim-based mode is
configured and that the IDP emits policies, returning a precise error
when either is missing. The existing error mapping below this point
surfaces those as InvalidParameterValue, matching what an AWS SDK
expects.

* test(iam): update missing-RoleArn STS integration test for the new contract

The previous commit drops the HTTP-layer RoleArn pre-check so claim-based
mode can derive the ARN from a JWT claim. The integration test still
asserted MissingParameter for the missing-RoleArn case, which now
reaches the STS layer and surfaces a JWT-parse error instead. Update
the assertion to match: missing RoleArn alone must no longer surface
as MissingParameter, but a bogus JWT must still be rejected.
This commit is contained in:
Chris Lu
2026-05-05 19:14:44 -07:00
committed by GitHub
parent 9af1b212d3
commit 12f283357f
8 changed files with 444 additions and 55 deletions
+12 -4
View File
@@ -74,25 +74,33 @@ func TestAssumeRoleWithWebIdentityValidation(t *testing.T) {
assert.Equal(t, "MissingParameter", errResp.Error.Code)
})
t.Run("missing_role_arn", func(t *testing.T) {
t.Run("missing_role_arn_invalid_jwt_still_rejected", func(t *testing.T) {
// Missing RoleArn is no longer a fast-fail at the HTTP layer:
// claim-based policy mode (Phase 3b) advertises RoleArn as
// optional so the STS service can derive the assumed-role ARN
// from the configured policy claim. Validation now happens at
// the STS layer once the JWT is parsed. With a bogus token the
// JWT parse fails first, so the request is still rejected —
// just with the JWT-parse error code instead of MissingParameter.
resp, err := callSTSAPI(t, url.Values{
"Action": {"AssumeRoleWithWebIdentity"},
"WebIdentityToken": {"fake-jwt-token"},
"RoleSessionName": {"test-session"},
// RoleArn is missing
// RoleArn omitted on purpose.
})
require.NoError(t, err)
defer resp.Body.Close()
assert.NotEqual(t, http.StatusOK, resp.StatusCode,
"Should fail without RoleArn")
"Should still fail when RoleArn is missing and the JWT is invalid")
body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
var errResp STSErrorTestResponse
err = xml.Unmarshal(body, &errResp)
require.NoError(t, err, "Failed to parse error response: %s", string(body))
assert.Equal(t, "MissingParameter", errResp.Error.Code)
assert.NotEqual(t, "MissingParameter", errResp.Error.Code,
"missing RoleArn alone must no longer surface as MissingParameter")
})
t.Run("missing_role_session_name", func(t *testing.T) {