From 1d3454ca5cdf4b3a6fb33f7e68de6a8e27799595 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 5 May 2026 12:21:55 -0700 Subject: [PATCH] feat(iam): claim-based policy mode for AssumeRoleWithWebIdentity (Phase 3b) (#9322) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(iam): claim-based policy mode for AssumeRoleWithWebIdentity When the caller passes the sentinel RoleArn arn:aws:iam:::role/sts-claim-based (or omits it entirely) and the matched OIDC provider has policyClaim set, mint a session whose effective policies come from that JWT claim instead of from a server-side role mapping. Accepts string, comma-separated string, or array shapes — MinIO-compatible behaviour for IDPs that already attach policies to the user. Trust-policy validation is skipped in claim-mode: the IDP is the sole authority for both authentication and authorization, mirroring the contract MinIO documents for its DummyRoleARN flow. Concrete-role mode is unchanged and still requires the role definition + trust policy. * fix(iam): trim policy-claim array elements + clean up stale comments Three medium-priority cleanups gemini flagged on the claim-based path: - extractClaimPolicies's array branch was leaving whitespace on each element while the string/comma-separated branch trimmed via splitPolicyClaimString. An IDP that emits ["readonly", " billing "] would create a "billing" policy lookup that didn't match the stored name. Trim every array element, drop empties. - The "synthetic ARN keyed on the session name" comment was wrong — effectiveRoleArn here is the literal sentinel; it's the assumed-role ARN generated downstream that's session-keyed. Reword. - The empty if/else block at the start of validateAssumeRoleWithWebIdentityRequest existed only to host a comment about deferred validation; the comment now lives in the function godoc and the empty branch is gone. Addresses three gemini medium reviews on PR #9322. --- weed/iam/oidc/oidc_provider.go | 64 +++++++++++++++++++++++ weed/iam/oidc/policy_claim_test.go | 57 ++++++++++++++++++++ weed/iam/providers/provider.go | 5 ++ weed/iam/sts/claim_based_policy_test.go | 23 +++++++++ weed/iam/sts/constants.go | 1 + weed/iam/sts/provider_factory.go | 4 ++ weed/iam/sts/sts_service.go | 69 ++++++++++++++++++++----- 7 files changed, 209 insertions(+), 14 deletions(-) create mode 100644 weed/iam/oidc/policy_claim_test.go create mode 100644 weed/iam/sts/claim_based_policy_test.go diff --git a/weed/iam/oidc/oidc_provider.go b/weed/iam/oidc/oidc_provider.go index 966a8f127..d85ef065e 100644 --- a/weed/iam/oidc/oidc_provider.go +++ b/weed/iam/oidc/oidc_provider.go @@ -92,6 +92,14 @@ type OIDCConfig struct { // allowlist (e.g. ["team", "env"]) to opt specific keys in. AllowedPrincipalTagKeys []string `json:"allowedPrincipalTagKeys,omitempty"` + // PolicyClaim names a JWT claim whose value carries the effective policy + // list for the session. When non-empty and the assume request opts into + // claim-based policy mode via the ClaimBasedPolicyRoleArn sentinel, the + // policies are pulled from this claim rather than from a server-side + // role mapping. Accepted shapes: string (single policy), comma-separated + // string, or string array. + PolicyClaim string `json:"policyClaim,omitempty"` + // TLSCACert is the path to the CA certificate file for custom/self-signed certificates TLSCACert string `json:"tlsCaCert,omitempty"` @@ -106,6 +114,61 @@ type OIDCConfig struct { // see https://docs.aws.amazon.com/IAM/latest/UserGuide/id_session-tags.html. const PrincipalTagsClaim = "https://aws.amazon.com/tags/principal_tags" +// extractClaimPolicies reads policy names from the configured JWT claim. +// Accepts three shapes: a single string ("readonly"), a comma-separated +// string ("readonly,billing"), or a string array. Returns nil when the +// provider isn't in claim-based mode or the claim is absent/empty. +func extractClaimPolicies(claims map[string]interface{}, claimName string) []string { + if claimName == "" { + return nil + } + raw, ok := claims[claimName] + if !ok { + return nil + } + switch v := raw.(type) { + case string: + return splitPolicyClaimString(v) + case []interface{}: + out := make([]string, 0, len(v)) + for _, e := range v { + s, ok := e.(string) + if !ok { + continue + } + s = strings.TrimSpace(s) + if s == "" { + continue + } + out = append(out, s) + } + if len(out) == 0 { + return nil + } + return out + } + return nil +} + +func splitPolicyClaimString(s string) []string { + if s == "" { + return nil + } + parts := strings.Split(s, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.TrimSpace(p) + if p == "" { + continue + } + out = append(out, p) + } + if len(out) == 0 { + return nil + } + return out +} + // 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. @@ -475,6 +538,7 @@ func (p *OIDCProvider) Authenticate(ctx context.Context, token string) (*provide Provider: p.name, Issuer: claims.Issuer, PrincipalTags: filterPrincipalTags(extractPrincipalTags(claims.Claims), p.config.AllowedPrincipalTagKeys), + ClaimPolicies: extractClaimPolicies(claims.Claims, p.config.PolicyClaim), } // Pass the token expiration to limit session duration diff --git a/weed/iam/oidc/policy_claim_test.go b/weed/iam/oidc/policy_claim_test.go new file mode 100644 index 000000000..cf8024e23 --- /dev/null +++ b/weed/iam/oidc/policy_claim_test.go @@ -0,0 +1,57 @@ +package oidc + +import ( + "reflect" + "testing" +) + +func TestExtractClaimPoliciesString(t *testing.T) { + got := extractClaimPolicies(map[string]interface{}{"policy": "readonly"}, "policy") + if !reflect.DeepEqual(got, []string{"readonly"}) { + t.Fatalf("got=%v", got) + } +} + +func TestExtractClaimPoliciesCommaSeparated(t *testing.T) { + got := extractClaimPolicies(map[string]interface{}{"policy": "readonly, billing , "}, "policy") + if !reflect.DeepEqual(got, []string{"readonly", "billing"}) { + t.Fatalf("got=%v", got) + } +} + +func TestExtractClaimPoliciesArray(t *testing.T) { + got := extractClaimPolicies(map[string]interface{}{ + "policy": []interface{}{"readonly", "billing", 42, ""}, // non-string + empty filtered + }, "policy") + if !reflect.DeepEqual(got, []string{"readonly", "billing"}) { + t.Fatalf("got=%v", got) + } +} + +func TestExtractClaimPoliciesMissing(t *testing.T) { + if got := extractClaimPolicies(map[string]interface{}{}, "policy"); got != nil { + t.Fatalf("expected nil, got %v", got) + } +} + +func TestExtractClaimPoliciesEmptyClaimName(t *testing.T) { + // Provider not in claim-mode -> never read the claim, even if present. + if got := extractClaimPolicies(map[string]interface{}{"policy": "readonly"}, ""); got != nil { + t.Fatalf("empty claim name should return nil, got %v", got) + } +} + +func TestExtractClaimPoliciesUnsupportedShape(t *testing.T) { + // Numeric / object / bool values should be ignored, not panic. + cases := []interface{}{ + 42, + 3.14, + map[string]interface{}{"x": "y"}, + true, + } + for _, v := range cases { + if got := extractClaimPolicies(map[string]interface{}{"policy": v}, "policy"); got != nil { + t.Fatalf("value %v: expected nil, got %v", v, got) + } + } +} diff --git a/weed/iam/providers/provider.go b/weed/iam/providers/provider.go index f2cc172c5..3c4fe5088 100644 --- a/weed/iam/providers/provider.go +++ b/weed/iam/providers/provider.go @@ -60,6 +60,11 @@ type ExternalIdentity struct { // (subject to per-provider allowlist filtering at the STS layer). PrincipalTags map[string]string `json:"principalTags,omitempty"` + // ClaimPolicies are policy names pulled from the provider-configured + // PolicyClaim. Empty when the provider isn't running in claim-based + // policy mode or the claim was absent. + ClaimPolicies []string `json:"claimPolicies,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/claim_based_policy_test.go b/weed/iam/sts/claim_based_policy_test.go new file mode 100644 index 000000000..60d7203e6 --- /dev/null +++ b/weed/iam/sts/claim_based_policy_test.go @@ -0,0 +1,23 @@ +package sts + +import "testing" + +func TestIsClaimBasedPolicyRoleArn(t *testing.T) { + cases := []struct { + name string + arn string + want bool + }{ + {"empty matches", "", true}, + {"sentinel matches", ClaimBasedPolicyRoleArn, true}, + {"concrete role does not", "arn:aws:iam::123:role/admin", false}, + {"random ARN does not", "arn:aws:sts::123:assumed-role/X/Y", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := IsClaimBasedPolicyRoleArn(tc.arn); got != tc.want { + t.Fatalf("got=%v want=%v", got, tc.want) + } + }) + } +} diff --git a/weed/iam/sts/constants.go b/weed/iam/sts/constants.go index ae45be041..9a4876069 100644 --- a/weed/iam/sts/constants.go +++ b/weed/iam/sts/constants.go @@ -49,6 +49,7 @@ const ( ConfigFieldClientIDs = "clientIds" ConfigFieldThumbprints = "thumbprints" ConfigFieldAllowedPrincipalTagKeys = "allowedPrincipalTagKeys" + ConfigFieldPolicyClaim = "policyClaim" ConfigFieldClientSecret = "clientSecret" ConfigFieldJWKSUri = "jwksUri" ConfigFieldScopes = "scopes" diff --git a/weed/iam/sts/provider_factory.go b/weed/iam/sts/provider_factory.go index c16b3a9c8..63e27e812 100644 --- a/weed/iam/sts/provider_factory.go +++ b/weed/iam/sts/provider_factory.go @@ -146,6 +146,10 @@ func (f *ProviderFactory) convertToOIDCConfig(configMap map[string]interface{}) } } + if policyClaim, ok := configMap[ConfigFieldPolicyClaim].(string); ok { + config.PolicyClaim = policyClaim + } + 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 7c1f7f1e8..24dc066ba 100644 --- a/weed/iam/sts/sts_service.go +++ b/weed/iam/sts/sts_service.go @@ -69,6 +69,21 @@ func (fd FlexibleDuration) MarshalJSON() ([]byte, error) { return json.Marshal(fd.Duration.String()) } +// ClaimBasedPolicyRoleArn is the AWS-shaped sentinel ARN that callers pass +// in the AssumeRoleWithWebIdentity RoleArn field to opt into claim-based +// policy resolution. Using a recognisable role-style ARN here (rather than +// requiring an empty RoleArn) lets SDKs and AWS CLI builds that always +// require RoleArn to be set still reach this code path. +const ClaimBasedPolicyRoleArn = "arn:aws:iam:::role/sts-claim-based" + +// IsClaimBasedPolicyRoleArn reports whether the supplied RoleArn opts the +// caller into claim-based policy mode. Either the empty string (caller +// omitted RoleArn entirely; common for SDKs that didn't expect to need one) +// or the explicit sentinel triggers the mode. +func IsClaimBasedPolicyRoleArn(arn string) bool { + return arn == "" || arn == ClaimBasedPolicyRoleArn +} + // STSService provides Security Token Service functionality // This service is now completely stateless - all session information is embedded // in JWT tokens, eliminating the need for session storage and enabling true @@ -505,17 +520,39 @@ func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *Ass return nil, fmt.Errorf("failed to validate web identity token: %w", err) } - // 2. Check if the role exists and can be assumed (includes trust policy validation) - if err := s.validateRoleAssumptionForWebIdentity(ctx, request.RoleArn, request.WebIdentityToken, request.DurationSeconds); err != nil { - return nil, fmt.Errorf("role assumption denied: %w", err) + // 2. Decide between concrete-role mode and claim-based policy mode. + // Claim-based mode requires both the sentinel RoleArn and a non-empty + // ClaimPolicies list — the second check guards against an IDP that just + // happens not to emit the policy claim today. + claimMode := IsClaimBasedPolicyRoleArn(request.RoleArn) && len(externalIdentity.ClaimPolicies) > 0 + effectiveRoleArn := request.RoleArn + if claimMode { + // Replace an empty RoleArn with the constant sentinel so the assumed- + // role ARN GenerateAssumedRoleArn produces below carries a stable, + // session-name-keyed principal that policy evaluation can log and + // rate-limit on. + effectiveRoleArn = ClaimBasedPolicyRoleArn + } else if request.RoleArn == "" { + return nil, fmt.Errorf("RoleArn is required when claim-based policy mode is not configured") + } else if request.RoleArn == ClaimBasedPolicyRoleArn { + return nil, fmt.Errorf("claim-based policy mode requires the IDP to emit policies via the configured policyClaim") } - // 3. Calculate session duration, capping at the source token's expiration + // 3. Trust-policy validation only runs in concrete-role mode. In + // claim-mode the IDP is the sole authority for both authentication and + // authorization, so there is no role definition to consult. + if !claimMode { + if err := s.validateRoleAssumptionForWebIdentity(ctx, request.RoleArn, request.WebIdentityToken, request.DurationSeconds); err != nil { + return nil, fmt.Errorf("role assumption denied: %w", err) + } + } + + // 4. Calculate session duration, capping at the source token's expiration // This ensures sessions from short-lived tokens (e.g., GitLab CI job tokens) don't outlive their source sessionDuration := s.calculateSessionDuration(request.DurationSeconds, externalIdentity.TokenExpiration) expiresAt := time.Now().Add(sessionDuration) - // 4. Generate session ID and credentials + // 5. Generate session ID and credentials sessionId, err := GenerateSessionId() if err != nil { return nil, fmt.Errorf("failed to generate session ID: %w", err) @@ -527,10 +564,10 @@ func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *Ass return nil, fmt.Errorf("failed to generate credentials: %w", err) } - // 5. Create comprehensive JWT session token with all session information embedded + // 6. Create comprehensive JWT session token with all session information embedded assumedRoleUser := &AssumedRoleUser{ - AssumedRoleId: request.RoleArn, - Arn: GenerateAssumedRoleArn(request.RoleArn, request.RoleSessionName), + AssumedRoleId: effectiveRoleArn, + Arn: GenerateAssumedRoleArn(effectiveRoleArn, request.RoleSessionName), Subject: externalIdentity.UserID, } @@ -576,10 +613,13 @@ func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *Ass // Create rich JWT claims with all session information sessionClaims := NewSTSSessionClaims(sessionId, s.Config.Issuer, expiresAt). WithSessionName(request.RoleSessionName). - WithRoleInfo(request.RoleArn, assumedRoleUser.Arn, assumedRoleUser.Arn). + WithRoleInfo(effectiveRoleArn, assumedRoleUser.Arn, assumedRoleUser.Arn). WithIdentityProvider(provider.Name(), externalIdentity.UserID, externalIdentity.Issuer). WithMaxDuration(sessionDuration). WithRequestContext(requestContext) + if claimMode { + sessionClaims.WithPolicies(externalIdentity.ClaimPolicies) + } if parentUser != "" { sessionClaims.WithParentUser(parentUser) } @@ -731,12 +771,13 @@ func (s *STSService) ValidateSessionToken(ctx context.Context, sessionToken stri // Helper methods for AssumeRoleWithWebIdentity -// validateAssumeRoleWithWebIdentityRequest validates the request parameters +// validateAssumeRoleWithWebIdentityRequest validates the request parameters. +// +// RoleArn validation lives in AssumeRoleWithWebIdentity itself rather than +// here: once we've parsed the JWT we know whether the caller has +// ClaimPolicies and can decide between concrete-role mode and claim-mode, +// which in turn determines whether an empty/sentinel RoleArn is acceptable. func (s *STSService) validateAssumeRoleWithWebIdentityRequest(request *AssumeRoleWithWebIdentityRequest) error { - if request.RoleArn == "" { - return fmt.Errorf("RoleArn is required") - } - if request.WebIdentityToken == "" { return fmt.Errorf("WebIdentityToken is required") }