diff --git a/test/s3/iam/s3_sts_test.go b/test/s3/iam/s3_sts_test.go index 84daf27a9..fdfcfafcd 100644 --- a/test/s3/iam/s3_sts_test.go +++ b/test/s3/iam/s3_sts_test.go @@ -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) { diff --git a/weed/iam/integration/iam_manager.go b/weed/iam/integration/iam_manager.go index 48e146aa8..e19eec2c3 100644 --- a/weed/iam/integration/iam_manager.go +++ b/weed/iam/integration/iam_manager.go @@ -585,12 +585,15 @@ func (m *IAMManager) initOIDCProviderStore(config *IAMConfig) error { createdAt = existing.CreatedAt } rec := &OIDCProviderRecord{ - AccountID: accountID, - ARN: arn, - URL: issuer, - ClientIDs: clientIDs, - CreatedAt: createdAt, - UpdatedAt: now, + AccountID: accountID, + ARN: arn, + URL: issuer, + ClientIDs: clientIDs, + Thumbprints: extractStringList(pc.Config, "thumbprints"), + AllowedPrincipalTagKeys: extractStringList(pc.Config, "allowedPrincipalTagKeys"), + PolicyClaim: extractString(pc.Config, "policyClaim"), + CreatedAt: createdAt, + UpdatedAt: now, } if err := store.StoreProvider(ctx, m.getFilerAddress(), rec); err != nil { glog.Warningf("mirror static OIDC provider %s into store: %v", pc.Name, err) @@ -625,7 +628,7 @@ func (m *IAMManager) RefreshOIDCProvidersFromStore(ctx context.Context) error { if err != nil { return fmt.Errorf("list OIDC providers: %w", err) } - byIssuer := make(map[string]providers.IdentityProvider, len(records)) + byIssuer := make(map[string][]sts.ScopedOIDCProvider, len(records)) for _, rec := range records { if rec == nil || rec.URL == "" { continue @@ -635,12 +638,15 @@ func (m *IAMManager) RefreshOIDCProvidersFromStore(ctx context.Context) error { glog.Warningf("skip refreshing OIDC provider %s: %v", rec.ARN, err) continue } - // Last write wins on issuer collision; the store is the source of - // truth, and an operator who has two records with the same issuer - // has already accepted one will shadow the other. - byIssuer[rec.URL] = provider + // Multiple records may share an issuer when each is scoped to a + // different account; STS picks the right one at validation time + // based on the role being assumed. See lookupOIDCProviderForAccount. + byIssuer[rec.URL] = append(byIssuer[rec.URL], sts.ScopedOIDCProvider{ + AccountID: rec.AccountID, + Provider: provider, + }) } - m.stsService.SetIAMManagedOIDCProvidersByIssuer(byIssuer) + m.stsService.SetIAMManagedOIDCProviders(byIssuer) return nil } @@ -698,6 +704,39 @@ func extractClientIDs(cfg map[string]interface{}) []string { return nil } +// extractStringList reads a JSON string array out of the provider's static +// config map and returns the non-empty entries. Returns nil when the key is +// missing, the value is the wrong shape, or every entry is empty. +func extractStringList(cfg map[string]interface{}, key string) []string { + if cfg == nil { + return nil + } + list, ok := cfg[key].([]interface{}) + if !ok { + return nil + } + out := make([]string, 0, len(list)) + for _, v := range list { + if s, ok := v.(string); ok && s != "" { + out = append(out, s) + } + } + if len(out) == 0 { + return nil + } + return out +} + +// extractString reads a single string field from the provider's static +// config map; missing or non-string values produce "". +func extractString(cfg map[string]interface{}, key string) string { + if cfg == nil { + return "" + } + s, _ := cfg[key].(string) + return s +} + // getFilerAddress returns the current filer address using the provider function func (m *IAMManager) getFilerAddress() string { if m.filerAddressProvider != nil { diff --git a/weed/iam/integration/oidc_provider_bootstrap_test.go b/weed/iam/integration/oidc_provider_bootstrap_test.go index 06a79c6a3..e98bb344c 100644 --- a/weed/iam/integration/oidc_provider_bootstrap_test.go +++ b/weed/iam/integration/oidc_provider_bootstrap_test.go @@ -102,6 +102,71 @@ func TestStaticConfigSeedsProviderStore(t *testing.T) { } } +func TestStaticConfigMirrorsThumbprintsAndAdvancedFields(t *testing.T) { + // Static config sets thumbprints, AllowedPrincipalTagKeys, and PolicyClaim; + // the mirror into the IAM-managed store must carry all three. Without it, + // the next RefreshOIDCProvidersFromStore rebuilds a runtime provider that + // drops thumbprint pinning and silently disables claim-based policies and + // principal-tag passthrough — and because IAM-managed entries take + // precedence over the static-config map, the bootstrap provider gets + // shadowed by this weaker rebuild. + mgr := NewIAMManager() + cfg := &IAMConfig{ + STS: &sts.STSConfig{ + TokenDuration: sts.FlexibleDuration{Duration: time.Hour}, + MaxSessionLength: sts.FlexibleDuration{Duration: 12 * time.Hour}, + Issuer: "test-sts", + SigningKey: []byte("test-signing-key-32-characters-long"), + AccountId: "111122223333", + Providers: []*sts.ProviderConfig{ + { + Name: "github-actions", + Type: sts.ProviderTypeOIDC, + Enabled: true, + Config: map[string]interface{}{ + "issuer": "https://token.actions.githubusercontent.com", + "clientId": "sts.amazonaws.com", + "thumbprints": []interface{}{"6938fd4d98bab03faadb97b34396831e3780aea1"}, + "allowedPrincipalTagKeys": []interface{}{"team", "env"}, + "policyClaim": "policies", + }, + }, + }, + }, + Policy: &policy.PolicyEngineConfig{DefaultEffect: "Deny", StoreType: "memory"}, + Roles: &RoleStoreConfig{StoreType: "memory"}, + } + if err := mgr.Initialize(cfg, func() string { return "localhost:8888" }); err != nil { + t.Fatalf("Initialize: %v", err) + } + + rec, err := mgr.GetOIDCProvider(context.Background(), "arn:aws:iam::111122223333:oidc-provider/token.actions.githubusercontent.com") + if err != nil { + t.Fatalf("GetOIDCProvider: %v", err) + } + if got, want := rec.Thumbprints, []string{"6938fd4d98bab03faadb97b34396831e3780aea1"}; !equalStrings(got, want) { + t.Fatalf("thumbprints mismatch: got %v want %v", got, want) + } + if got, want := rec.AllowedPrincipalTagKeys, []string{"team", "env"}; !equalStrings(got, want) { + t.Fatalf("allowedPrincipalTagKeys mismatch: got %v want %v", got, want) + } + if rec.PolicyClaim != "policies" { + t.Fatalf("policyClaim mismatch: got %q want %q", rec.PolicyClaim, "policies") + } +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + func TestStoreNotConfiguredReturnsClearError(t *testing.T) { mgr := NewIAMManager() if _, err := mgr.GetOIDCProvider(context.Background(), "arn:..."); err == nil { diff --git a/weed/iam/sts/oidc_provider_lookup_test.go b/weed/iam/sts/oidc_provider_lookup_test.go new file mode 100644 index 000000000..0281e65b3 --- /dev/null +++ b/weed/iam/sts/oidc_provider_lookup_test.go @@ -0,0 +1,105 @@ +package sts + +import ( + "context" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/iam/providers" +) + +// stubIdentityProvider is the minimal IdentityProvider needed to drive +// lookupOIDCProviderForAccount; the lookup never calls Authenticate. +type stubIdentityProvider struct{ name string } + +func (s *stubIdentityProvider) Name() string { return s.name } +func (s *stubIdentityProvider) Initialize(interface{}) error { return nil } +func (s *stubIdentityProvider) Authenticate(context.Context, string) (*providers.ExternalIdentity, error) { + return nil, nil +} +func (s *stubIdentityProvider) GetUserInfo(context.Context, string) (*providers.ExternalIdentity, error) { + return nil, nil +} +func (s *stubIdentityProvider) ValidateToken(context.Context, string) (*providers.TokenClaims, error) { + return nil, nil +} + +func TestLookupOIDCProviderForAccountPrefersAccountMatch(t *testing.T) { + const issuer = "https://example.com" + accountA := &stubIdentityProvider{name: "A"} + accountB := &stubIdentityProvider{name: "B"} + + s := &STSService{issuerToProvider: map[string]providers.IdentityProvider{}} + s.SetIAMManagedOIDCProviders(map[string][]ScopedOIDCProvider{ + issuer: { + {AccountID: "111111111111", Provider: accountA}, + {AccountID: "222222222222", Provider: accountB}, + }, + }) + + got, ok := s.lookupOIDCProviderForAccount(issuer, "222222222222") + if !ok || got != accountB { + t.Fatalf("expected accountB provider for matching account, got %v ok=%v", got, ok) + } + got, ok = s.lookupOIDCProviderForAccount(issuer, "111111111111") + if !ok || got != accountA { + t.Fatalf("expected accountA provider for matching account, got %v ok=%v", got, ok) + } +} + +func TestLookupOIDCProviderForAccountFallsBackToGlobal(t *testing.T) { + const issuer = "https://example.com" + global := &stubIdentityProvider{name: "global"} + accountA := &stubIdentityProvider{name: "A"} + + s := &STSService{issuerToProvider: map[string]providers.IdentityProvider{}} + s.SetIAMManagedOIDCProviders(map[string][]ScopedOIDCProvider{ + issuer: { + {AccountID: "", Provider: global}, + {AccountID: "111111111111", Provider: accountA}, + }, + }) + + // Account not represented in records → global match. + got, ok := s.lookupOIDCProviderForAccount(issuer, "999999999999") + if !ok || got != global { + t.Fatalf("expected global provider as fallback, got %v ok=%v", got, ok) + } + // Empty account hint → never picks an account-scoped record arbitrarily; + // only the global record is eligible. + got, ok = s.lookupOIDCProviderForAccount(issuer, "") + if !ok || got != global { + t.Fatalf("expected global provider when account unknown, got %v ok=%v", got, ok) + } +} + +func TestLookupOIDCProviderForAccountSkipsAccountSpecificWhenAccountUnknown(t *testing.T) { + const issuer = "https://example.com" + accountA := &stubIdentityProvider{name: "A"} + + s := &STSService{issuerToProvider: map[string]providers.IdentityProvider{}} + s.SetIAMManagedOIDCProviders(map[string][]ScopedOIDCProvider{ + issuer: { + {AccountID: "111111111111", Provider: accountA}, + }, + }) + + // No account hint AND no global record → must fall through (return false), + // not silently pick the account-A entry. Picking an arbitrary entry is the + // pre-fix bug that lets a token be validated by the wrong tenant's record. + if _, ok := s.lookupOIDCProviderForAccount(issuer, ""); ok { + t.Fatalf("expected no match when account unknown and only account-scoped records exist") + } +} + +func TestLookupOIDCProviderForAccountFallsBackToStatic(t *testing.T) { + const issuer = "https://example.com" + static := &stubIdentityProvider{name: "static"} + + s := &STSService{issuerToProvider: map[string]providers.IdentityProvider{issuer: static}} + s.SetIAMManagedOIDCProviders(nil) + + got, ok := s.lookupOIDCProviderForAccount(issuer, "111111111111") + if !ok || got != static { + t.Fatalf("expected static-config provider as last resort, got %v ok=%v", got, ok) + } +} diff --git a/weed/iam/sts/sts_service.go b/weed/iam/sts/sts_service.go index 24dc066ba..1d520af2f 100644 --- a/weed/iam/sts/sts_service.go +++ b/weed/iam/sts/sts_service.go @@ -98,13 +98,28 @@ type STSService struct { // iamManagedOIDCMu guards iamManagedOIDCByIssuer. The map is the live view // of providers persisted in the IAM-managed OIDCProviderStore; it is - // atomically replaced by SetIAMManagedOIDCProvidersByIssuer whenever the - // store changes (either via a local IAM API call or a metadata-subscribe - // event from a peer). Lookups consult this map first and fall back to the + // atomically replaced by SetIAMManagedOIDCProviders whenever the store + // changes (either via a local IAM API call or a metadata-subscribe event + // from a peer). Lookups consult this map first and fall back to the // static-config issuerToProvider so admin-managed entries always take // precedence over the bootstrap config. - iamManagedOIDCMu sync.RWMutex - iamManagedOIDCByIssuer map[string]providers.IdentityProvider + // + // The slice value lets multiple records share an issuer when each is + // scoped to a different account; lookup picks the entry whose AccountID + // matches the role being assumed (or the global, AccountID="" entry as a + // fallback). Without this, two accounts' records for the same issuer + // would race for one map slot and a token could be validated by a + // provider that wasn't scoped to the role's account. + iamManagedOIDCMu sync.RWMutex + iamManagedOIDCByIssuer map[string][]ScopedOIDCProvider +} + +// ScopedOIDCProvider pairs an OIDC IdentityProvider with the account it is +// scoped to. AccountID="" means the provider is global (usable from any +// account). +type ScopedOIDCProvider struct { + AccountID string + Provider providers.IdentityProvider } // GetTokenGenerator returns the token generator used by the STS service. @@ -450,36 +465,65 @@ func (s *STSService) GetProviders() map[string]providers.IdentityProvider { return s.providers } -// SetIAMManagedOIDCProvidersByIssuer atomically replaces the IAM-managed -// OIDC provider map. Pass nil or an empty map to clear all managed entries. -// The caller passes a fully-built map keyed by issuer URL; the STS service -// copies the reference and serves AssumeRoleWithWebIdentity lookups from it -// in preference to the static-config issuerToProvider map. -func (s *STSService) SetIAMManagedOIDCProvidersByIssuer(byIssuer map[string]providers.IdentityProvider) { +// SetIAMManagedOIDCProviders atomically replaces the IAM-managed OIDC +// provider map. Pass nil or an empty map to clear all managed entries. The +// caller passes a fully-built map keyed by issuer URL; the slice value lets +// per-account records coexist under the same issuer. +func (s *STSService) SetIAMManagedOIDCProviders(byIssuer map[string][]ScopedOIDCProvider) { // Defensively copy so callers can keep mutating their map without affecting // in-flight lookups. A nil input becomes an empty map for cheap reads. - cp := make(map[string]providers.IdentityProvider, len(byIssuer)) - for k, v := range byIssuer { - if k == "" || v == nil { + cp := make(map[string][]ScopedOIDCProvider, len(byIssuer)) + for issuer, scoped := range byIssuer { + if issuer == "" { continue } - cp[k] = v + entries := make([]ScopedOIDCProvider, 0, len(scoped)) + for _, sp := range scoped { + if sp.Provider == nil { + continue + } + entries = append(entries, sp) + } + if len(entries) > 0 { + cp[issuer] = entries + } } s.iamManagedOIDCMu.Lock() s.iamManagedOIDCByIssuer = cp s.iamManagedOIDCMu.Unlock() } -// lookupOIDCProviderByIssuer returns the provider that should validate tokens -// from `issuer`, consulting the IAM-managed map first and falling back to the -// static-config map. Returns ok=false when no provider is registered. -func (s *STSService) lookupOIDCProviderByIssuer(issuer string) (providers.IdentityProvider, bool) { +// lookupOIDCProviderForAccount returns the provider that should validate +// tokens from `issuer` when the caller is assuming a role in `accountID`. +// Selection order: +// 1. IAM-managed record exactly scoped to accountID (when accountID != ""); +// 2. IAM-managed record with empty AccountID (global); +// 3. static-config issuerToProvider (legacy path; account-agnostic). +// +// Without the (issuer, account) key, two records for the same issuer (e.g. +// account A with clientIDs=[a] and account B with clientIDs=[b]) would race +// for one map slot, and a token destined for account B could be validated by +// account A's record. The role-account check in +// IAMManager.enforceProviderAccountScope blocks the cross-account assumption +// itself, but the validation must use the right record's clientIDs and +// thumbprints in the first place. +func (s *STSService) lookupOIDCProviderForAccount(issuer, accountID string) (providers.IdentityProvider, bool) { s.iamManagedOIDCMu.RLock() - if p, ok := s.iamManagedOIDCByIssuer[issuer]; ok { - s.iamManagedOIDCMu.RUnlock() - return p, true + scoped := s.iamManagedOIDCByIssuer[issuer] + var globalMatch providers.IdentityProvider + for _, sp := range scoped { + if accountID != "" && sp.AccountID == accountID { + s.iamManagedOIDCMu.RUnlock() + return sp.Provider, true + } + if sp.AccountID == "" && globalMatch == nil { + globalMatch = sp.Provider + } } s.iamManagedOIDCMu.RUnlock() + if globalMatch != nil { + return globalMatch, true + } p, ok := s.issuerToProvider[issuer] return p, ok } @@ -514,8 +558,13 @@ func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *Ass sessionPolicy = normalized } - // 1. Validate the web identity token with appropriate provider - externalIdentity, provider, err := s.validateWebIdentityToken(ctx, request.WebIdentityToken) + // 1. Validate the web identity token with appropriate provider. The role + // ARN's account scopes which IAM-managed record may validate the token — + // see lookupOIDCProviderForAccount. ParseRoleARN returns "" when the + // caller passed a legacy or claim-based ARN, in which case lookup falls + // back to a global (account-less) record only. + roleAccountID := utils.ParseRoleARN(request.RoleArn).AccountID + externalIdentity, provider, err := s.validateWebIdentityToken(ctx, request.WebIdentityToken, roleAccountID) if err != nil { return nil, fmt.Errorf("failed to validate web identity token: %w", err) } @@ -799,7 +848,7 @@ func (s *STSService) validateAssumeRoleWithWebIdentityRequest(request *AssumeRol // validateWebIdentityToken validates the web identity token with strict issuer-to-provider mapping // SECURITY: JWT tokens with a specific issuer claim MUST only be validated by the provider for that issuer // SECURITY: This method only accepts JWT tokens. Non-JWT authentication must use AssumeRoleWithCredentials with explicit ProviderName. -func (s *STSService) validateWebIdentityToken(ctx context.Context, token string) (*providers.ExternalIdentity, providers.IdentityProvider, error) { +func (s *STSService) validateWebIdentityToken(ctx context.Context, token, roleAccountID string) (*providers.ExternalIdentity, providers.IdentityProvider, error) { // Try to extract issuer from JWT token for strict validation issuer, err := s.extractIssuerFromJWT(token) if err != nil { @@ -810,11 +859,12 @@ func (s *STSService) validateWebIdentityToken(ctx context.Context, token string) return nil, nil, fmt.Errorf("web identity token must be a valid JWT token: %w", err) } - // Look up the specific provider for this issuer. IAM-managed records - // (admin-controlled, mutable at runtime) take precedence over the - // static-config map so an operator's CreateOpenIDConnectProvider call - // can shadow a bootstrap entry without requiring a restart. - provider, exists := s.lookupOIDCProviderByIssuer(issuer) + // Look up the specific provider for this issuer, scoped to the role's + // account when known. IAM-managed records (admin-controlled, mutable at + // runtime) take precedence over the static-config map so an operator's + // CreateOpenIDConnectProvider call can shadow a bootstrap entry without + // requiring a restart. + provider, exists := s.lookupOIDCProviderForAccount(issuer, roleAccountID) if !exists { // SECURITY: If no provider is registered for this issuer, fail immediately // This prevents JWT tokens from being validated by unintended providers @@ -849,9 +899,19 @@ func (s *STSService) validateWebIdentityToken(ctx context.Context, token string) } // ValidateWebIdentityToken is a public method that exposes secure token validation for external use -// This method uses issuer-based lookup to select the correct provider, ensuring security and efficiency +// This method uses issuer-based lookup to select the correct provider, ensuring security and efficiency. +// External callers without role context get the account-agnostic lookup (global IAM-managed records +// only, then static-config); call ValidateWebIdentityTokenForAccount when the assumed-role account +// is known. func (s *STSService) ValidateWebIdentityToken(ctx context.Context, token string) (*providers.ExternalIdentity, providers.IdentityProvider, error) { - return s.validateWebIdentityToken(ctx, token) + return s.validateWebIdentityToken(ctx, token, "") +} + +// ValidateWebIdentityTokenForAccount mirrors ValidateWebIdentityToken but +// scopes the IAM-managed provider lookup to roleAccountID. Pass "" for +// callers that don't yet know the account (e.g. claim-based mode). +func (s *STSService) ValidateWebIdentityTokenForAccount(ctx context.Context, token, roleAccountID string) (*providers.ExternalIdentity, providers.IdentityProvider, error) { + return s.validateWebIdentityToken(ctx, token, roleAccountID) } // extractIssuerFromJWT extracts the issuer (iss) claim from a JWT token without verification diff --git a/weed/s3api/s3api_sts.go b/weed/s3api/s3api_sts.go index de9f38c0f..baee534c7 100644 --- a/weed/s3api/s3api_sts.go +++ b/weed/s3api/s3api_sts.go @@ -161,6 +161,22 @@ func (h *STSHandlers) getAccountID() string { return defaultAccountID } +// assumeRoleWithWebIdentity dispatches the request through the IAMManager +// wrapper when one is wired so its cross-account provider scope check and +// per-role MaxSessionDuration clamp run for the public AWS-SDK path. Without +// this dispatch, both checks are silently skipped because they live on the +// IAMManager, not on the bare STS service. +func (h *STSHandlers) assumeRoleWithWebIdentity(ctx context.Context, request *sts.AssumeRoleWithWebIdentityRequest) (*sts.AssumeRoleResponse, error) { + if h.iam != nil && h.iam.iamIntegration != nil { + if provider, ok := h.iam.iamIntegration.(IAMManagerProvider); ok { + if mgr := provider.GetIAMManager(); mgr != nil { + return mgr.AssumeRoleWithWebIdentity(ctx, request) + } + } + } + return h.stsService.AssumeRoleWithWebIdentity(ctx, request) +} + // HandleSTSRequest is the main entry point for STS requests // It routes requests based on the Action parameter func (h *STSHandlers) HandleSTSRequest(w http.ResponseWriter, r *http.Request) { @@ -213,11 +229,13 @@ func (h *STSHandlers) handleAssumeRoleWithWebIdentity(w http.ResponseWriter, r * return } - if roleArn == "" { - h.writeSTSErrorResponse(w, r, STSErrMissingParameter, - fmt.Errorf("RoleArn is required")) - return - } + // RoleArn is intentionally optional here: claim-based policy mode + // (Phase 3b) advertises that callers MAY omit RoleArn so the STS + // service derives the assumed-role ARN from the configured policy + // claim. The bare-STS path validates this — when the IDP isn't + // configured for claim-based mode (or fails to emit policies) it + // returns a precise error that this handler maps to the right STS + // error code below. if errCode, err := validateRoleSessionName(roleSessionName); err != nil { h.writeSTSErrorResponse(w, r, errCode, err) @@ -259,8 +277,11 @@ func (h *STSHandlers) handleAssumeRoleWithWebIdentity(w http.ResponseWriter, r * Policy: sessionPolicyPtr, } - // Call STS service - response, err := h.stsService.AssumeRoleWithWebIdentity(ctx, request) + // Prefer the IAMManager wrapper so the cross-account provider scope + // (enforceProviderAccountScope) and per-role MaxSessionDuration clamp + // run for SDK callers too. Falling back to the bare STS service keeps + // embedded test setups (no IAM integration wired) working. + response, err := h.assumeRoleWithWebIdentity(ctx, request) if err != nil { glog.V(2).Infof("AssumeRoleWithWebIdentity failed: %v", err) diff --git a/weed/s3api/s3api_sts_assume_role_dispatch_test.go b/weed/s3api/s3api_sts_assume_role_dispatch_test.go new file mode 100644 index 000000000..a9f2d333c --- /dev/null +++ b/weed/s3api/s3api_sts_assume_role_dispatch_test.go @@ -0,0 +1,49 @@ +package s3api + +import ( + "context" + "strings" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/iam/integration" + "github.com/seaweedfs/seaweedfs/weed/iam/sts" +) + +// TestAssumeRoleWithWebIdentity_DispatchesThroughIAMManager confirms the +// public STS HTTP path goes through IAMManager.AssumeRoleWithWebIdentity (and +// thereby its enforceProviderAccountScope check and MaxSessionDuration +// clamp) instead of bypassing to the bare STS service. The two paths surface +// different errors when their underlying service is uninitialized, which is +// the cheapest behavioural signal that doesn't require a full OIDC stack. +func TestAssumeRoleWithWebIdentity_DispatchesThroughIAMManager(t *testing.T) { + req := &sts.AssumeRoleWithWebIdentityRequest{ + RoleArn: "arn:aws:iam::111111111111:role/Test", + WebIdentityToken: "ignored", + RoleSessionName: "session", + } + + t.Run("with IAMManager wired", func(t *testing.T) { + mgr := integration.NewIAMManager() // not initialized on purpose + h := &STSHandlers{ + stsService: nil, // intentionally nil; the wrapper must run first + iam: &IdentityAccessManagement{ + iamIntegration: NewS3IAMIntegration(mgr, ""), + }, + } + _, err := h.assumeRoleWithWebIdentity(context.Background(), req) + if err == nil || !strings.Contains(err.Error(), "IAM manager not initialized") { + t.Fatalf("expected IAMManager wrapper to handle the call; got err=%v", err) + } + }) + + t.Run("without IAM integration", func(t *testing.T) { + h := &STSHandlers{ + stsService: sts.NewSTSService(), // not initialized + iam: &IdentityAccessManagement{}, + } + _, err := h.assumeRoleWithWebIdentity(context.Background(), req) + if err == nil || !strings.Contains(err.Error(), "STS service not initialized") { + t.Fatalf("expected fallback to bare STS service; got err=%v", err) + } + }) +} diff --git a/weed/s3api/s3api_sts_assume_role_empty_arn_test.go b/weed/s3api/s3api_sts_assume_role_empty_arn_test.go new file mode 100644 index 000000000..2d9b30888 --- /dev/null +++ b/weed/s3api/s3api_sts_assume_role_empty_arn_test.go @@ -0,0 +1,42 @@ +package s3api + +import ( + "net/http/httptest" + "net/url" + "strings" + "testing" +) + +// TestAssumeRoleWithWebIdentity_AllowsEmptyRoleArn confirms the HTTP handler +// no longer rejects empty RoleArn before STS sees the request. Phase 3b's +// claim-based mode advertises that callers MAY omit RoleArn so the policy +// claim derives the assumed-role ARN; the handler must let that flow +// through. STS-layer failures (invalid token, claim-mode not configured) are +// surfaced separately and don't read "RoleArn is required". +func TestAssumeRoleWithWebIdentity_AllowsEmptyRoleArn(t *testing.T) { + stsService, _ := setupTestSTSService(t) + h := &STSHandlers{ + stsService: stsService, + iam: &IdentityAccessManagement{}, + } + + form := url.Values{} + form.Set("Action", "AssumeRoleWithWebIdentity") + form.Set("WebIdentityToken", "not-a-real-jwt") + form.Set("RoleSessionName", "session-1") + // RoleArn intentionally omitted. + + req := httptest.NewRequest("POST", "/", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + if err := req.ParseForm(); err != nil { + t.Fatalf("ParseForm: %v", err) + } + + rr := httptest.NewRecorder() + h.handleAssumeRoleWithWebIdentity(rr, req) + + body := rr.Body.String() + if strings.Contains(body, "RoleArn is required") { + t.Fatalf("HTTP handler rejected empty RoleArn pre-STS; body=%q", body) + } +}