address comments

test/s3/iam/Makefile

@@ -87,7 +87,7 @@ start-services: ## Start SeaweedFS services for testing
 	@$(WEED_BINARY) -v=3 s3 -port=$(S3_PORT) \
 		-filer=localhost:$(FILER_PORT) \
 		-config=test_config.json \
-                -iam.config=$(shell pwd)/iam_config.json > weed-s3.log 2>&1 & \
+                -iam.config=$(CURDIR)/iam_config.json > weed-s3.log 2>&1 & \
 		echo $$! > $(S3_PID_FILE)
 	
 	@echo "✅ All services started"
@@ -234,9 +234,11 @@ docker-logs: ## Show logs from all services
 
 docker-test: docker-up ## Run tests with Docker Compose including Keycloak
 	@echo "🧪 Running Keycloak integration tests..."
+	@echo "⏳ Waiting for services to be ready..."
+	@timeout 120 bash -c 'until curl -s http://localhost:8080/health/ready > /dev/null 2>&1; do sleep 2; done' || (echo "❌ Keycloak failed to become ready" && exit 1)
+	@timeout 60 bash -c 'until curl -s http://localhost:8333 > /dev/null 2>&1; do sleep 2; done' || (echo "❌ S3 API failed to become ready" && exit 1)
 	@export KEYCLOAK_URL="http://localhost:8080" && \
 	 export S3_ENDPOINT="http://localhost:8333" && \
-	 sleep 10 && \
 	 go test -v -timeout $(TEST_TIMEOUT) -run "TestKeycloak" ./...
 	@echo "🐳 Stopping services after tests..."
 	@make docker-down
@@ -274,9 +276,11 @@ test-versioning-stress: ## Run S3 versioning stress tests
 
 test-keycloak-full: docker-up ## Run complete Keycloak integration tests
 	@echo "🔐 Running complete Keycloak integration tests..."
+	@echo "⏳ Waiting for services to be ready..."
+	@timeout 120 bash -c 'until curl -s http://localhost:8080/health/ready > /dev/null 2>&1; do sleep 2; done' || (echo "❌ Keycloak failed to become ready" && exit 1)
+	@timeout 60 bash -c 'until curl -s http://localhost:8333 > /dev/null 2>&1; do sleep 2; done' || (echo "❌ S3 API failed to become ready" && exit 1)
 	@export KEYCLOAK_URL="http://localhost:8080" && \
 	 export S3_ENDPOINT="http://localhost:8333" && \
-	 sleep 15 && \
 	 go test -v -timeout $(TEST_TIMEOUT) -run "TestKeycloak" ./...
 	@make docker-down
 

test/s3/iam/s3_iam_distributed_test.go

@@ -218,13 +218,11 @@ func TestS3IAMDistributedTests(t *testing.T) {
 		// STRICT CONCURRENCY TESTING: Use appropriate thresholds for the operation count
 		// For totalOperations=6, error thresholds need to be adjusted to avoid unreachable conditions
 
-		// Serious errors (race conditions, deadlocks) should be zero or very rare
-		maxSeriousErrors := 1 // Allow 1 serious error max (16.7%) for small test size
+		// Serious errors (race conditions, deadlocks) should be zero for reliable testing
+		maxSeriousErrors := 0 // Allow 0 serious errors max - any serious error indicates system issues
 		if len(seriousErrors) > maxSeriousErrors {
-			t.Errorf("❌ Too many serious errors: %d (%.1f%%) - max allowed: %d. This indicates potential race conditions, deadlocks, or other concurrency bugs.",
-				len(seriousErrors), seriousErrorRate*100, maxSeriousErrors)
-		} else if len(seriousErrors) > 0 {
-			t.Logf("⚠️  %d serious error detected (%.1f%%) - within threshold but should be investigated", len(seriousErrors), seriousErrorRate*100)
+			t.Errorf("❌ %d serious error(s) detected, indicating potential concurrency bugs. First error: %v",
+				len(seriousErrors), seriousErrors[0])
 		}
 
 		// For total errors, allow more flexibility with small operation counts

weed/iam/integration/iam_manager.go

@@ -95,6 +95,9 @@ func (m *IAMManager) Initialize(config *IAMConfig) error {
 		return fmt.Errorf("failed to initialize STS service: %w", err)
 	}
 
+	// CRITICAL SECURITY: Set trust policy validator to ensure proper role assumption validation
+	m.stsService.SetTrustPolicyValidator(m)
+
 	// Initialize policy engine
 	m.policyEngine = policy.NewPolicyEngine()
 	if err := m.policyEngine.Initialize(config.Policy); err != nil {
@@ -514,8 +517,6 @@ func (m *IAMManager) evaluateTrustPolicyConditions(conditions map[string]map[str
 	return true
 }
 
-
-
 // isOIDCToken checks if a token is an OIDC JWT token (vs STS session token)
 func isOIDCToken(token string) bool {
 	// JWT tokens have three parts separated by dots and start with base64-encoded JSON
@@ -527,3 +528,50 @@ func isOIDCToken(token string) bool {
 	// JWT tokens typically start with "eyJ" (base64 encoded JSON starting with "{")
 	return strings.HasPrefix(token, "eyJ")
 }
+
+// TrustPolicyValidator interface implementation
+// These methods allow the IAMManager to serve as the trust policy validator for the STS service
+
+// ValidateTrustPolicyForWebIdentity implements the TrustPolicyValidator interface
+func (m *IAMManager) ValidateTrustPolicyForWebIdentity(ctx context.Context, roleArn string, webIdentityToken string) error {
+	if !m.initialized {
+		return fmt.Errorf("IAM manager not initialized")
+	}
+
+	// Extract role name from ARN
+	roleName := extractRoleNameFromArn(roleArn)
+
+	// Get role definition
+	roleDef, err := m.roleStore.GetRole(ctx, "", roleName)
+	if err != nil {
+		return fmt.Errorf("role not found: %s", roleName)
+	}
+
+	// Use existing trust policy validation logic
+	return m.validateTrustPolicyForWebIdentity(ctx, roleDef, webIdentityToken)
+}
+
+// ValidateTrustPolicyForCredentials implements the TrustPolicyValidator interface
+func (m *IAMManager) ValidateTrustPolicyForCredentials(ctx context.Context, roleArn string, identity *providers.ExternalIdentity) error {
+	if !m.initialized {
+		return fmt.Errorf("IAM manager not initialized")
+	}
+
+	// Extract role name from ARN
+	roleName := extractRoleNameFromArn(roleArn)
+
+	// Get role definition
+	roleDef, err := m.roleStore.GetRole(ctx, "", roleName)
+	if err != nil {
+		return fmt.Errorf("role not found: %s", roleName)
+	}
+
+	// For credentials, we need to create a mock request to reuse existing validation
+	// This is a bit of a hack, but it allows us to reuse the existing logic
+	mockRequest := &sts.AssumeRoleWithCredentialsRequest{
+		ProviderName: identity.Provider, // Use the provider name from the identity
+	}
+
+	// Use existing trust policy validation logic
+	return m.validateTrustPolicyForCredentials(ctx, roleDef, mockRequest)
+}

weed/iam/sts/sts_service.go

@@ -10,6 +10,15 @@ import (
 	"github.com/seaweedfs/seaweedfs/weed/iam/providers"
 )
 
+// TrustPolicyValidator interface for validating trust policies during role assumption
+type TrustPolicyValidator interface {
+	// ValidateTrustPolicyForWebIdentity validates if a web identity token can assume a role
+	ValidateTrustPolicyForWebIdentity(ctx context.Context, roleArn string, webIdentityToken string) error
+
+	// ValidateTrustPolicyForCredentials validates if credentials can assume a role
+	ValidateTrustPolicyForCredentials(ctx context.Context, roleArn string, identity *providers.ExternalIdentity) error
+}
+
 // 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
@@ -20,6 +29,7 @@ type STSService struct {
 	providers            map[string]providers.IdentityProvider
 	issuerToProvider     map[string]providers.IdentityProvider // Efficient issuer-based provider lookup
 	tokenGenerator       *TokenGenerator
+	trustPolicyValidator TrustPolicyValidator // Interface for trust policy validation
 }
 
 // STSConfig holds STS service configuration
@@ -315,6 +325,11 @@ func (s *STSService) GetProviders() map[string]providers.IdentityProvider {
 	return s.providers
 }
 
+// SetTrustPolicyValidator sets the trust policy validator for role assumption validation
+func (s *STSService) SetTrustPolicyValidator(validator TrustPolicyValidator) {
+	s.trustPolicyValidator = validator
+}
+
 // AssumeRoleWithWebIdentity assumes a role using a web identity token (OIDC)
 // This method is now completely stateless - all session information is embedded in the JWT token
 func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *AssumeRoleWithWebIdentityRequest) (*AssumeRoleResponse, error) {
@@ -337,8 +352,8 @@ 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
-	if err := s.validateRoleAssumption(request.RoleArn, externalIdentity); err != nil {
+	// 2. Check if the role exists and can be assumed (includes trust policy validation)
+	if err := s.validateRoleAssumptionForWebIdentity(ctx, request.RoleArn, request.WebIdentityToken); err != nil {
 		return nil, fmt.Errorf("role assumption denied: %w", err)
 	}
 
@@ -416,8 +431,8 @@ func (s *STSService) AssumeRoleWithCredentials(ctx context.Context, request *Ass
 		return nil, fmt.Errorf("failed to authenticate credentials: %w", err)
 	}
 
-	// 3. Check if the role exists and can be assumed
-	if err := s.validateRoleAssumption(request.RoleArn, externalIdentity); err != nil {
+	// 3. Check if the role exists and can be assumed (includes trust policy validation)
+	if err := s.validateRoleAssumptionForCredentials(ctx, request.RoleArn, externalIdentity); err != nil {
 		return nil, fmt.Errorf("role assumption denied: %w", err)
 	}
 
@@ -583,14 +598,46 @@ func (s *STSService) extractIssuerFromJWT(token string) (string, error) {
 	return issuer, nil
 }
 
-// validateRoleAssumption checks if the role can be assumed by the external identity
-func (s *STSService) validateRoleAssumption(roleArn string, identity *providers.ExternalIdentity) error {
-	// For now, we'll do basic validation
-	// In a full implementation, this would check:
-	// 1. Role exists
-	// 2. Role trust policy allows assumption by this identity
-	// 3. Identity has permission to assume the role
+// validateRoleAssumptionForWebIdentity validates role assumption for web identity tokens
+// This method performs complete trust policy validation to prevent unauthorized role assumptions
+func (s *STSService) validateRoleAssumptionForWebIdentity(ctx context.Context, roleArn string, webIdentityToken string) error {
+	if roleArn == "" {
+		return fmt.Errorf("role ARN cannot be empty")
+	}
 
+	if webIdentityToken == "" {
+		return fmt.Errorf("web identity token cannot be empty")
+	}
+
+	// Basic role ARN format validation
+	expectedPrefix := "arn:seaweed:iam::role/"
+	if len(roleArn) < len(expectedPrefix) || roleArn[:len(expectedPrefix)] != expectedPrefix {
+		return fmt.Errorf("invalid role ARN format: got %s, expected format: %s*", roleArn, expectedPrefix)
+	}
+
+	// For testing, reject non-existent roles
+	roleName := extractRoleNameFromArn(roleArn)
+	if roleName == "NonExistentRole" {
+		return fmt.Errorf("role does not exist: %s", roleName)
+	}
+
+	// CRITICAL SECURITY: Perform trust policy validation
+	if s.trustPolicyValidator != nil {
+		if err := s.trustPolicyValidator.ValidateTrustPolicyForWebIdentity(ctx, roleArn, webIdentityToken); err != nil {
+			return fmt.Errorf("trust policy validation failed: %w", err)
+		}
+	} else {
+		// If no trust policy validator is configured, fail closed for security
+		glog.Errorf("SECURITY WARNING: No trust policy validator configured - denying role assumption for security")
+		return fmt.Errorf("trust policy validation not available - role assumption denied for security")
+	}
+
+	return nil
+}
+
+// validateRoleAssumptionForCredentials validates role assumption for credential-based authentication
+// This method performs complete trust policy validation to prevent unauthorized role assumptions
+func (s *STSService) validateRoleAssumptionForCredentials(ctx context.Context, roleArn string, identity *providers.ExternalIdentity) error {
 	if roleArn == "" {
 		return fmt.Errorf("role ARN cannot be empty")
 	}
@@ -611,6 +658,17 @@ func (s *STSService) validateRoleAssumption(roleArn string, identity *providers.
 		return fmt.Errorf("role does not exist: %s", roleName)
 	}
 
+	// CRITICAL SECURITY: Perform trust policy validation
+	if s.trustPolicyValidator != nil {
+		if err := s.trustPolicyValidator.ValidateTrustPolicyForCredentials(ctx, roleArn, identity); err != nil {
+			return fmt.Errorf("trust policy validation failed: %w", err)
+		}
+	} else {
+		// If no trust policy validator is configured, fail closed for security
+		glog.Errorf("SECURITY WARNING: No trust policy validator configured - denying role assumption for security")
+		return fmt.Errorf("trust policy validation not available - role assumption denied for security")
+	}
+
 	return nil
 }