From 6554ab7928ad9b3097ef2697ca5b48216eb0d4f1 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 5 May 2026 11:43:05 -0700 Subject: [PATCH] feat(iam): principal session tags from OIDC tokens (Phase 3a) (#9321) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(iam): principal session tags from OIDC tokens Extract the AWS principal-tags namespace claim (`https://aws.amazon.com/tags/principal_tags`) from validated OIDC tokens, filter through a per-provider AllowedPrincipalTagKeys allowlist, and surface as `aws:PrincipalTag/` in the STS session request context. Empty allowlist means "no tags surfaced" — operators must opt keys in explicitly so a misconfigured IDP can't pollute policy evaluation. Policy engine now accepts `aws:PrincipalTag/...` and `aws:RequestTag/...` as substitutable variables so resource-level ABAC policies can reference them. * fix(iam): case-insensitive principal-tag allowlist + sharper comment filterPrincipalTags compared keys case-sensitively, but AWS IAM session tag keys are case-insensitive (the docs are explicit). An IDP whose claim casing drifts from the operator-configured allowlist string would silently filter the value out — surprising failure mode. Lowercase both sides during the lookup; the original key casing is preserved on the output so policy variables still match what the caller sees. Also reword the "anything the IDP signs is acceptable" comment in sts_service.go: it predates the per-provider allowlist that's already filtering before this point. The reality is now "everything reaching here is on the operator's opt-in list, dropped entirely if the allowlist is empty." Addresses two gemini medium reviews on PR #9321. --- weed/iam/oidc/oidc_provider.go | 94 +++++++++++++++++++++++++--- weed/iam/oidc/principal_tags_test.go | 83 ++++++++++++++++++++++++ weed/iam/policy/policy_engine.go | 11 ++-- weed/iam/providers/provider.go | 6 ++ weed/iam/sts/constants.go | 1 + weed/iam/sts/provider_factory.go | 6 ++ weed/iam/sts/sts_service.go | 9 +++ 7 files changed, 199 insertions(+), 11 deletions(-) create mode 100644 weed/iam/oidc/principal_tags_test.go diff --git a/weed/iam/oidc/oidc_provider.go b/weed/iam/oidc/oidc_provider.go index 81e7c28a1..966a8f127 100644 --- a/weed/iam/oidc/oidc_provider.go +++ b/weed/iam/oidc/oidc_provider.go @@ -87,6 +87,11 @@ type OIDCConfig struct { // root store" (or whatever TLSCACert configures). Thumbprints []string `json:"thumbprints,omitempty"` + // AllowedPrincipalTagKeys filters the keys read from the AWS principal + // session tags claim. Empty means "no tags surfaced". Provide an explicit + // allowlist (e.g. ["team", "env"]) to opt specific keys in. + AllowedPrincipalTagKeys []string `json:"allowedPrincipalTagKeys,omitempty"` + // TLSCACert is the path to the CA certificate file for custom/self-signed certificates TLSCACert string `json:"tlsCaCert,omitempty"` @@ -95,6 +100,80 @@ type OIDCConfig struct { TLSInsecureSkipVerify bool `json:"tlsInsecureSkipVerify,omitempty"` } +// PrincipalTagsClaim is the AWS-defined namespace claim that carries +// principal session tags. Tokens that include this claim must encode it as +// an object whose top-level keys are tag names. AWS uses the same string; +// see https://docs.aws.amazon.com/IAM/latest/UserGuide/id_session-tags.html. +const PrincipalTagsClaim = "https://aws.amazon.com/tags/principal_tags" + +// filterPrincipalTags drops keys that are not on `allowed`. An empty +// allowlist means "deny all" — security-conservative default that forces +// operators to explicitly opt tags in. Returns nil when the result is empty. +// +// Comparison is case-insensitive on the key, matching AWS IAM's session-tag +// rules (the AWS docs explicitly state tag keys are case-insensitive even +// though the original casing is preserved on the value side). Without this +// an IDP whose claim casing drifts from the operator's allowlist string +// would fail in surprising ways. +func filterPrincipalTags(tags map[string]string, allowed []string) map[string]string { + if len(tags) == 0 { + return nil + } + if len(allowed) == 0 { + return nil + } + allowSet := make(map[string]struct{}, len(allowed)) + for _, k := range allowed { + allowSet[strings.ToLower(k)] = struct{}{} + } + out := make(map[string]string, len(tags)) + for k, v := range tags { + if _, ok := allowSet[strings.ToLower(k)]; ok { + out[k] = v + } + } + if len(out) == 0 { + return nil + } + return out +} + +// extractPrincipalTags pulls the principal-tags namespace claim out of the +// JWT claim map. Only string values survive — everything else is dropped to +// avoid surfacing structured data into a flat policy condition key. Returns +// nil when the claim is absent or empty. +func extractPrincipalTags(claims map[string]interface{}) map[string]string { + raw, ok := claims[PrincipalTagsClaim] + if !ok { + return nil + } + obj, ok := raw.(map[string]interface{}) + if !ok || len(obj) == 0 { + return nil + } + out := make(map[string]string, len(obj)) + for k, v := range obj { + switch s := v.(type) { + case string: + out[k] = s + case []interface{}: + // Multi-value tag: AWS condition keys carry only a single value, so + // take the first stringy element. Multi-value matching can land + // later if a real customer needs it. + for _, e := range s { + if str, ok := e.(string); ok { + out[k] = str + break + } + } + } + } + if len(out) == 0 { + return nil + } + return out +} + // normalizeThumbprints lowercases and de-duplicates the configured allowlist. // Returns a set keyed by lowercase hex for O(1) lookup during TLS verification. func normalizeThumbprints(in []string) map[string]struct{} { @@ -388,13 +467,14 @@ func (p *OIDCProvider) Authenticate(ctx context.Context, token string) (*provide } identity := &providers.ExternalIdentity{ - UserID: claims.Subject, - Email: email, - DisplayName: displayName, - Groups: groups, - Attributes: attributes, - Provider: p.name, - Issuer: claims.Issuer, + UserID: claims.Subject, + Email: email, + DisplayName: displayName, + Groups: groups, + Attributes: attributes, + Provider: p.name, + Issuer: claims.Issuer, + PrincipalTags: filterPrincipalTags(extractPrincipalTags(claims.Claims), p.config.AllowedPrincipalTagKeys), } // Pass the token expiration to limit session duration diff --git a/weed/iam/oidc/principal_tags_test.go b/weed/iam/oidc/principal_tags_test.go new file mode 100644 index 000000000..6444dec26 --- /dev/null +++ b/weed/iam/oidc/principal_tags_test.go @@ -0,0 +1,83 @@ +package oidc + +import ( + "reflect" + "testing" +) + +func TestExtractPrincipalTagsObjectShape(t *testing.T) { + claims := map[string]interface{}{ + PrincipalTagsClaim: map[string]interface{}{ + "team": "infra", + "env": "prod", + "empty": "", + // non-string values are dropped + "count": 42, + }, + } + got := extractPrincipalTags(claims) + want := map[string]string{"team": "infra", "env": "prod", "empty": ""} + if !reflect.DeepEqual(got, want) { + t.Fatalf("got=%v want=%v", got, want) + } +} + +func TestExtractPrincipalTagsArrayValueTakesFirst(t *testing.T) { + claims := map[string]interface{}{ + PrincipalTagsClaim: map[string]interface{}{ + "team": []interface{}{"infra", "ignored"}, + }, + } + got := extractPrincipalTags(claims) + if got["team"] != "infra" { + t.Fatalf("expected first array element, got %q", got["team"]) + } +} + +func TestExtractPrincipalTagsAbsent(t *testing.T) { + if got := extractPrincipalTags(map[string]interface{}{}); got != nil { + t.Fatalf("expected nil for missing claim, got %v", got) + } +} + +func TestExtractPrincipalTagsWrongShape(t *testing.T) { + claims := map[string]interface{}{PrincipalTagsClaim: "not-an-object"} + if got := extractPrincipalTags(claims); got != nil { + t.Fatalf("expected nil for non-object claim, got %v", got) + } +} + +func TestFilterPrincipalTagsAllowlist(t *testing.T) { + in := map[string]string{"team": "infra", "env": "prod", "secret": "shh"} + + t.Run("empty allowlist denies all", func(t *testing.T) { + if got := filterPrincipalTags(in, nil); got != nil { + t.Fatalf("expected nil, got %v", got) + } + }) + + t.Run("partial allowlist", func(t *testing.T) { + got := filterPrincipalTags(in, []string{"team", "env"}) + want := map[string]string{"team": "infra", "env": "prod"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("got=%v want=%v", got, want) + } + }) + + t.Run("allowlist with no matches", func(t *testing.T) { + if got := filterPrincipalTags(in, []string{"unknown"}); got != nil { + t.Fatalf("expected nil, got %v", got) + } + }) + + t.Run("case-insensitive on key", func(t *testing.T) { + // AWS session tag keys are case-insensitive. An IDP whose claim + // uses different casing than the operator's allowlist string + // must still match. + got := filterPrincipalTags(map[string]string{"Team": "infra"}, []string{"TEAM"}) + want := map[string]string{"Team": "infra"} // original casing preserved on output + if !reflect.DeepEqual(got, want) { + t.Fatalf("got=%v want=%v", got, want) + } + }) +} diff --git a/weed/iam/policy/policy_engine.go b/weed/iam/policy/policy_engine.go index 6810d09b3..acd7c0a2f 100644 --- a/weed/iam/policy/policy_engine.go +++ b/weed/iam/policy/policy_engine.go @@ -48,16 +48,19 @@ var ( // isSafePolicyVariable reports whether a policy variable may be substituted // with a value from RequestContext. The fixed allowlist covers AWS-defined -// variables; any jwt:/saml:/oidc: claim is also allowed because those come -// from a validated identity token (the STS session JWT or federated assertion) -// and the claim set is controlled by the trusted identity provider. +// variables; any jwt:/saml:/oidc:/aws:PrincipalTag/ claim is also allowed +// because those come from a validated identity token (the STS session JWT +// or federated assertion) and the claim set is controlled by the trusted +// identity provider. func isSafePolicyVariable(variable string) bool { if safePolicyVariables[variable] { return true } return strings.HasPrefix(variable, "jwt:") || strings.HasPrefix(variable, "saml:") || - strings.HasPrefix(variable, "oidc:") + strings.HasPrefix(variable, "oidc:") || + strings.HasPrefix(variable, "aws:PrincipalTag/") || + strings.HasPrefix(variable, "aws:RequestTag/") } // PolicyEngine evaluates policies against requests diff --git a/weed/iam/providers/provider.go b/weed/iam/providers/provider.go index f9fac3dae..f2cc172c5 100644 --- a/weed/iam/providers/provider.go +++ b/weed/iam/providers/provider.go @@ -54,6 +54,12 @@ type ExternalIdentity struct { // a stable parent-user hash that survives token rotation. Issuer string `json:"issuer,omitempty"` + // PrincipalTags are key/value pairs extracted from the AWS principal-tags + // namespace claim (`https://aws.amazon.com/tags/principal_tags`). They are + // surfaced as `aws:PrincipalTag/` in the policy request context + // (subject to per-provider allowlist filtering at the STS layer). + PrincipalTags map[string]string `json:"principalTags,omitempty"` + // TokenExpiration is the expiration time of the source identity token // This is used to limit session duration to not exceed the token's exp claim TokenExpiration *time.Time `json:"tokenExpiration,omitempty"` diff --git a/weed/iam/sts/constants.go b/weed/iam/sts/constants.go index 65b8b1ffd..ae45be041 100644 --- a/weed/iam/sts/constants.go +++ b/weed/iam/sts/constants.go @@ -48,6 +48,7 @@ const ( ConfigFieldClientID = "clientId" ConfigFieldClientIDs = "clientIds" ConfigFieldThumbprints = "thumbprints" + ConfigFieldAllowedPrincipalTagKeys = "allowedPrincipalTagKeys" ConfigFieldClientSecret = "clientSecret" ConfigFieldJWKSUri = "jwksUri" ConfigFieldScopes = "scopes" diff --git a/weed/iam/sts/provider_factory.go b/weed/iam/sts/provider_factory.go index 44dc858b6..c16b3a9c8 100644 --- a/weed/iam/sts/provider_factory.go +++ b/weed/iam/sts/provider_factory.go @@ -140,6 +140,12 @@ func (f *ProviderFactory) convertToOIDCConfig(configMap map[string]interface{}) } } + if rawAllowed, ok := configMap[ConfigFieldAllowedPrincipalTagKeys]; ok { + if list, err := f.convertToStringSlice(rawAllowed); err == nil { + config.AllowedPrincipalTagKeys = list + } + } + if tlsInsecureSkipVerify, ok := configMap[ConfigFieldTLSInsecureSkipVerify].(bool); ok { config.TLSInsecureSkipVerify = tlsInsecureSkipVerify } diff --git a/weed/iam/sts/sts_service.go b/weed/iam/sts/sts_service.go index bf3ac2f82..7c1f7f1e8 100644 --- a/weed/iam/sts/sts_service.go +++ b/weed/iam/sts/sts_service.go @@ -564,6 +564,15 @@ func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *Ass requestContext["aws:userid"] = parentUser } + // Surface principal session tags as aws:PrincipalTag/. The OIDC + // provider has already filtered the claim namespace through its + // AllowedPrincipalTagKeys list (see filterPrincipalTags), so anything + // reaching us here is on the operator's opt-in list. The full claim + // is dropped if the allowlist is empty, which is the secure default. + for k, v := range externalIdentity.PrincipalTags { + requestContext["aws:PrincipalTag/"+k] = v + } + // Create rich JWT claims with all session information sessionClaims := NewSTSSessionClaims(sessionId, s.Config.Issuer, expiresAt). WithSessionName(request.RoleSessionName).