address comments

test/s3/iam/s3_iam_distributed_test.go

@@ -132,7 +132,7 @@ func TestS3IAMDistributedTests(t *testing.T) {
 			var lastErr error
 			for attempt := 0; attempt <= maxRetries; attempt++ {
 				if attempt > 0 {
-					time.Sleep(retryDelay * time.Duration(attempt)) // Exponential backoff
+					time.Sleep(retryDelay * time.Duration(attempt)) // Linear backoff
 				}
 
 				lastErr = operation()

test/s3/iam/s3_iam_framework.go

@@ -390,9 +390,8 @@ func (f *S3IAMTestFramework) generateSTSSessionToken(username, roleName string,
 		return "", err
 	}
 
-	// Note: In a real implementation, we would also create the session in the STS session store
-	// For now, we'll rely on the fact that if JWT validation passes, the session should be considered valid
-	// This is a limitation of our current testing approach
+	// The generated JWT is self-contained and includes all necessary session information.
+	// The stateless design of the STS service means no external session storage is required.
 
 	return tokenString, nil
 }

weed/iam/integration/role_store.go

@@ -317,11 +317,21 @@ func (f *FilerRoleStore) DeleteRole(ctx context.Context, filerAddress string, ro
 		}
 
 		glog.V(3).Infof("Deleting role %s", roleName)
-		_, err := client.DeleteEntry(ctx, request)
+		resp, err := client.DeleteEntry(ctx, request)
 		if err != nil {
+			if strings.Contains(err.Error(), "not found") {
+				return nil // Idempotent: deletion of non-existent role is successful
+			}
 			return fmt.Errorf("failed to delete role %s: %v", roleName, err)
 		}
 
+		if resp.Error != "" {
+			if strings.Contains(resp.Error, "not found") {
+				return nil // Idempotent: deletion of non-existent role is successful
+			}
+			return fmt.Errorf("failed to delete role %s: %s", roleName, resp.Error)
+		}
+
 		return nil
 	})
 }

weed/iam/sts/provider_factory.go

@@ -133,23 +133,29 @@ func (f *ProviderFactory) convertToOIDCConfig(configMap map[string]interface{})
 
 	// Convert scopes array
 	if scopesInterface, ok := configMap[ConfigFieldScopes]; ok {
-		if scopes, err := f.convertToStringSlice(scopesInterface); err == nil {
-			config.Scopes = scopes
+		scopes, err := f.convertToStringSlice(scopesInterface)
+		if err != nil {
+			return nil, fmt.Errorf("failed to convert scopes: %w", err)
 		}
+		config.Scopes = scopes
 	}
 
 	// Convert claims mapping
 	if claimsMapInterface, ok := configMap["claimsMapping"]; ok {
-		if claimsMap, err := f.convertToStringMap(claimsMapInterface); err == nil {
-			config.ClaimsMapping = claimsMap
+		claimsMap, err := f.convertToStringMap(claimsMapInterface)
+		if err != nil {
+			return nil, fmt.Errorf("failed to convert claimsMapping: %w", err)
 		}
+		config.ClaimsMapping = claimsMap
 	}
 
 	// Convert role mapping
 	if roleMappingInterface, ok := configMap["roleMapping"]; ok {
-		if roleMapping, err := f.convertToRoleMapping(roleMappingInterface); err == nil {
-			config.RoleMapping = roleMapping
+		roleMapping, err := f.convertToRoleMapping(roleMappingInterface)
+		if err != nil {
+			return nil, fmt.Errorf("failed to convert roleMapping: %w", err)
 		}
+		config.RoleMapping = roleMapping
 	}
 
 	glog.V(3).Infof("Converted OIDC config: issuer=%s, clientId=%s, jwksUri=%s",

weed/iam/sts/provider_factory_test.go

@@ -198,6 +198,64 @@ func TestProviderFactory_ConvertToStringSlice(t *testing.T) {
 	})
 }
 
+func TestProviderFactory_ConfigConversionErrors(t *testing.T) {
+	factory := NewProviderFactory()
+
+	t.Run("invalid scopes type", func(t *testing.T) {
+		config := &ProviderConfig{
+			Name:    "invalid-scopes",
+			Type:    "oidc",
+			Enabled: true,
+			Config: map[string]interface{}{
+				"issuer":   "https://test-issuer.com",
+				"clientId": "test-client",
+				"scopes":   "invalid-not-array", // Should be array
+			},
+		}
+
+		provider, err := factory.CreateProvider(config)
+		assert.Error(t, err)
+		assert.Nil(t, provider)
+		assert.Contains(t, err.Error(), "failed to convert scopes")
+	})
+
+	t.Run("invalid claimsMapping type", func(t *testing.T) {
+		config := &ProviderConfig{
+			Name:    "invalid-claims",
+			Type:    "oidc",
+			Enabled: true,
+			Config: map[string]interface{}{
+				"issuer":        "https://test-issuer.com",
+				"clientId":      "test-client",
+				"claimsMapping": "invalid-not-map", // Should be map
+			},
+		}
+
+		provider, err := factory.CreateProvider(config)
+		assert.Error(t, err)
+		assert.Nil(t, provider)
+		assert.Contains(t, err.Error(), "failed to convert claimsMapping")
+	})
+
+	t.Run("invalid roleMapping type", func(t *testing.T) {
+		config := &ProviderConfig{
+			Name:    "invalid-roles",
+			Type:    "oidc",
+			Enabled: true,
+			Config: map[string]interface{}{
+				"issuer":      "https://test-issuer.com",
+				"clientId":    "test-client",
+				"roleMapping": "invalid-not-map", // Should be map
+			},
+		}
+
+		provider, err := factory.CreateProvider(config)
+		assert.Error(t, err)
+		assert.Nil(t, provider)
+		assert.Contains(t, err.Error(), "failed to convert roleMapping")
+	})
+}
+
 func TestProviderFactory_ConvertToStringMap(t *testing.T) {
 	factory := NewProviderFactory()
 

weed/iam/sts/sts_service.go

@@ -641,8 +641,13 @@ func (s *STSService) validateRoleAssumptionForWebIdentity(ctx context.Context, r
 		return fmt.Errorf("invalid role ARN format: got %s, expected format: %s*", roleArn, expectedPrefix)
 	}
 
-	// For testing, reject non-existent roles
+	// Extract role name and validate ARN format
 	roleName := extractRoleNameFromArn(roleArn)
+	if roleName == "" {
+		return fmt.Errorf("invalid role ARN format: %s", roleArn)
+	}
+
+	// For testing, reject non-existent roles
 	if roleName == "NonExistentRole" {
 		return fmt.Errorf("role does not exist: %s", roleName)
 	}
@@ -678,8 +683,13 @@ func (s *STSService) validateRoleAssumptionForCredentials(ctx context.Context, r
 		return fmt.Errorf("invalid role ARN format: got %s, expected format: %s*", roleArn, expectedPrefix)
 	}
 
-	// For testing, reject non-existent roles
+	// Extract role name and validate ARN format
 	roleName := extractRoleNameFromArn(roleArn)
+	if roleName == "" {
+		return fmt.Errorf("invalid role ARN format: %s", roleArn)
+	}
+
+	// For testing, reject non-existent roles
 	if roleName == "NonExistentRole" {
 		return fmt.Errorf("role does not exist: %s", roleName)
 	}

weed/iam/sts/token_utils.go

@@ -6,6 +6,7 @@ import (
 	"encoding/base64"
 	"encoding/hex"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/golang-jwt/jwt/v5"
@@ -207,15 +208,20 @@ func GenerateSessionId() (string, error) {
 func GenerateAssumedRoleArn(roleArn, sessionName string) string {
 	// Convert role ARN to assumed role user ARN
 	// arn:seaweed:iam::role/RoleName -> arn:seaweed:sts::assumed-role/RoleName/SessionName
-	return fmt.Sprintf("arn:seaweed:sts::assumed-role/%s/%s", extractRoleNameFromArn(roleArn), sessionName)
+	roleName := extractRoleNameFromArn(roleArn)
+	if roleName == "" {
+		// This should not happen if validation is done properly upstream
+		return fmt.Sprintf("arn:seaweed:sts::assumed-role/INVALID-ARN/%s", sessionName)
+	}
+	return fmt.Sprintf("arn:seaweed:sts::assumed-role/%s/%s", roleName, sessionName)
 }
 
 // extractRoleNameFromArn extracts the role name from a role ARN
 func extractRoleNameFromArn(roleArn string) string {
 	// Simple extraction for arn:seaweed:iam::role/RoleName
 	prefix := "arn:seaweed:iam::role/"
-	if len(roleArn) > len(prefix) && roleArn[:len(prefix)] == prefix {
+	if strings.HasPrefix(roleArn, prefix) && len(roleArn) > len(prefix) {
 		return roleArn[len(prefix):]
 	}
-	return "UnknownRole"
+	return ""
 }

weed/iam/utils/arn_utils.go

@@ -23,6 +23,7 @@ func ExtractRoleNameFromPrincipal(principal string) string {
 		return principal[len(iamPrefix):]
 	}
 
-	// For backwards compatibility, return original principal if no recognized format
-	return principal
+	// Return empty string to signal invalid ARN format
+	// This allows callers to handle the error explicitly instead of masking it
+	return ""
 }

weed/s3api/s3_iam_simple_test.go

@@ -451,7 +451,7 @@ func TestExtractRoleNameFromPrincipal(t *testing.T) {
 		{
 			name:      "invalid format",
 			principal: "invalid-principal",
-			expected:  "invalid-principal", // Returns original on failure
+			expected:  "", // Returns empty string to signal invalid format
 		},
 		{
 			name:      "missing session name",