diff --git a/test/s3/iam/Makefile b/test/s3/iam/Makefile index c026339c9..7b4358ca0 100644 --- a/test/s3/iam/Makefile +++ b/test/s3/iam/Makefile @@ -6,7 +6,7 @@ all: test # Test configuration -WEED_BINARY ?= /Users/chrislu/go/bin/weed +WEED_BINARY ?= $(shell go env GOPATH)/bin/weed LOG_LEVEL ?= 2 S3_PORT ?= 8333 FILER_PORT ?= 8888 diff --git a/weed/iam/integration/iam_manager.go b/weed/iam/integration/iam_manager.go index cc0cd66e0..14622f198 100644 --- a/weed/iam/integration/iam_manager.go +++ b/weed/iam/integration/iam_manager.go @@ -5,12 +5,12 @@ import ( "encoding/base64" "encoding/json" "fmt" - "path/filepath" "strings" "github.com/seaweedfs/seaweedfs/weed/iam/policy" "github.com/seaweedfs/seaweedfs/weed/iam/providers" "github.com/seaweedfs/seaweedfs/weed/iam/sts" + "github.com/seaweedfs/seaweedfs/weed/iam/utils" ) // IAMManager orchestrates all IAM components @@ -240,7 +240,7 @@ func (m *IAMManager) IsActionAllowed(ctx context.Context, request *ActionRequest } // Extract role name from principal ARN - roleName := extractRoleNameFromPrincipal(request.Principal) + roleName := utils.ExtractRoleNameFromPrincipal(request.Principal) if roleName == "" { return false, fmt.Errorf("could not extract role from principal: %s", request.Principal) } @@ -381,37 +381,6 @@ func extractRoleNameFromArn(roleArn string) string { return "" } -// extractRoleNameFromPrincipal extracts role name from principal ARN -func extractRoleNameFromPrincipal(principal string) string { - // Handle STS assumed role format: arn:seaweed:sts::assumed-role/RoleName/SessionName - stsPrefix := "arn:seaweed:sts::assumed-role/" - if len(principal) > len(stsPrefix) && principal[:len(stsPrefix)] == stsPrefix { - remainder := principal[len(stsPrefix):] - // Split on first '/' to get role name - if slashIndex := indexOf(remainder, "/"); slashIndex != -1 { - return remainder[:slashIndex] - } - } - - // Handle IAM role format: arn:seaweed:iam::role/RoleName - iamPrefix := "arn:seaweed:iam::role/" - if len(principal) > len(iamPrefix) && principal[:len(iamPrefix)] == iamPrefix { - return principal[len(iamPrefix):] - } - - return "" -} - -// indexOf finds the index of the first occurrence of substring in string -func indexOf(s, substr string) int { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return i - } - } - return -1 -} - // ExpireSessionForTesting manually expires a session for testing purposes func (m *IAMManager) ExpireSessionForTesting(ctx context.Context, sessionToken string) error { if !m.initialized { @@ -525,15 +494,15 @@ func (m *IAMManager) evaluateTrustPolicyConditions(conditions map[string]map[str for conditionType, conditionBlock := range conditions { switch conditionType { case "StringEquals": - if !m.evaluateStringConditionForTrust(conditionBlock, evalCtx, true, false) { + if !m.policyEngine.EvaluateStringCondition(conditionBlock, evalCtx, true, false) { return false } case "StringNotEquals": - if !m.evaluateStringConditionForTrust(conditionBlock, evalCtx, false, false) { + if !m.policyEngine.EvaluateStringCondition(conditionBlock, evalCtx, false, false) { return false } case "StringLike": - if !m.evaluateStringConditionForTrust(conditionBlock, evalCtx, true, true) { + if !m.policyEngine.EvaluateStringCondition(conditionBlock, evalCtx, true, true) { return false } // Add other condition types as needed @@ -545,71 +514,7 @@ func (m *IAMManager) evaluateTrustPolicyConditions(conditions map[string]map[str return true } -// evaluateStringConditionForTrust evaluates string conditions for trust policies -func (m *IAMManager) evaluateStringConditionForTrust(block map[string]interface{}, evalCtx *policy.EvaluationContext, shouldMatch bool, useWildcard bool) bool { - for conditionKey, conditionValue := range block { - contextValues, exists := evalCtx.RequestContext[conditionKey] - if !exists { - if shouldMatch { - return false - } - continue - } - - // Convert context value to string slice - var contextStrings []string - switch v := contextValues.(type) { - case string: - contextStrings = []string{v} - case []string: - contextStrings = v - default: - contextStrings = []string{fmt.Sprintf("%v", v)} - } - - // Convert condition value to string slice - var expectedStrings []string - switch v := conditionValue.(type) { - case string: - expectedStrings = []string{v} - case []string: - expectedStrings = v - default: - expectedStrings = []string{fmt.Sprintf("%v", v)} - } - - // Evaluate the condition - conditionMet := false - for _, expected := range expectedStrings { - for _, contextValue := range contextStrings { - if useWildcard { - matched, err := filepath.Match(expected, contextValue) - if err == nil && matched { - conditionMet = true - break - } - } else { - if expected == contextValue { - conditionMet = true - break - } - } - } - if conditionMet { - break - } - } - - if shouldMatch && !conditionMet { - return false - } - if !shouldMatch && conditionMet { - return false - } - } - return true -} // isOIDCToken checks if a token is an OIDC JWT token (vs STS session token) func isOIDCToken(token string) bool { diff --git a/weed/iam/policy/policy_engine.go b/weed/iam/policy/policy_engine.go index 8d000b8b9..1b76cb47c 100644 --- a/weed/iam/policy/policy_engine.go +++ b/weed/iam/policy/policy_engine.go @@ -360,11 +360,11 @@ func (e *PolicyEngine) evaluateConditionBlock(conditionType string, block map[st case "NotIpAddress": return e.evaluateIPCondition(block, evalCtx, false) case "StringEquals": - return e.evaluateStringCondition(block, evalCtx, true, false) + return e.EvaluateStringCondition(block, evalCtx, true, false) case "StringNotEquals": - return e.evaluateStringCondition(block, evalCtx, false, false) + return e.EvaluateStringCondition(block, evalCtx, false, false) case "StringLike": - return e.evaluateStringCondition(block, evalCtx, true, true) + return e.EvaluateStringCondition(block, evalCtx, true, true) default: // Unknown condition types default to false (more secure) return false @@ -418,8 +418,8 @@ func (e *PolicyEngine) evaluateIPCondition(block map[string]interface{}, evalCtx return !shouldMatch } -// evaluateStringCondition evaluates string-based conditions -func (e *PolicyEngine) evaluateStringCondition(block map[string]interface{}, evalCtx *EvaluationContext, shouldMatch bool, useWildcard bool) bool { +// EvaluateStringCondition evaluates string-based conditions +func (e *PolicyEngine) EvaluateStringCondition(block map[string]interface{}, evalCtx *EvaluationContext, shouldMatch bool, useWildcard bool) bool { // Iterate through all condition keys in the block for conditionKey, conditionValue := range block { // Get the context values for this condition key @@ -582,38 +582,37 @@ func validateStatementWithType(statement *Statement, policyType string) error { return nil } + // matchResource checks if a resource pattern matches a requested resource +// Uses filepath.Match for consistent wildcard behavior across the IAM system func matchResource(pattern, resource string) bool { if pattern == resource { return true } - if pattern == "*" { - return true - } - - if strings.HasSuffix(pattern, "*") { - prefix := pattern[:len(pattern)-1] - return strings.HasPrefix(resource, prefix) + // Use filepath.Match for standard wildcard support (*, ?, []) + matched, err := filepath.Match(pattern, resource) + if err != nil { + // Fallback to exact match if pattern is malformed + return pattern == resource } - return false + return matched } // matchAction checks if an action pattern matches a requested action +// Uses filepath.Match for consistent wildcard behavior across the IAM system func matchAction(pattern, action string) bool { if pattern == action { return true } - if pattern == "*" { - return true - } - - if strings.HasSuffix(pattern, "*") { - prefix := pattern[:len(pattern)-1] - return strings.HasPrefix(action, prefix) + // Use filepath.Match for standard wildcard support (*, ?, []) + matched, err := filepath.Match(pattern, action) + if err != nil { + // Fallback to exact match if pattern is malformed + return pattern == action } - return false + return matched } diff --git a/weed/iam/sts/constants.go b/weed/iam/sts/constants.go index c684b45fe..4cd30366d 100644 --- a/weed/iam/sts/constants.go +++ b/weed/iam/sts/constants.go @@ -30,11 +30,11 @@ const ( // Default Values const ( - DefaultTokenDuration = 3600 // 1 hour in seconds - DefaultMaxSessionLength = 43200 // 12 hours in seconds - DefaultIssuer = "seaweedfs-sts" - DefaultStoreType = StoreTypeFiler // Default store type for persistence - MinSigningKeyLength = 16 // Minimum signing key length in bytes + DefaultTokenDuration = 3600 // 1 hour in seconds + DefaultMaxSessionLength = 43200 // 12 hours in seconds + DefaultIssuer = "seaweedfs-sts" + DefaultStoreType = StoreTypeFiler // Default store type for persistence + MinSigningKeyLength = 16 // Minimum signing key length in bytes ) // Configuration Field Names @@ -52,30 +52,30 @@ const ( // Error Messages const ( - ErrConfigCannotBeNil = "config cannot be nil" - ErrProviderCannotBeNil = "provider cannot be nil" - ErrProviderNameEmpty = "provider name cannot be empty" - ErrProviderTypeEmpty = "provider type cannot be empty" - ErrTokenCannotBeEmpty = "token cannot be empty" - ErrSessionTokenCannotBeEmpty = "session token cannot be empty" - ErrSessionIDCannotBeEmpty = "session ID cannot be empty" - ErrSTSServiceNotInitialized = "STS service not initialized" - ErrProviderNotInitialized = "provider not initialized" - ErrInvalidTokenDuration = "token duration must be positive" - ErrInvalidMaxSessionLength = "max session length must be positive" - ErrIssuerRequired = "issuer is required" - ErrSigningKeyTooShort = "signing key must be at least %d bytes" - ErrFilerAddressRequired = "filer address is required" - ErrClientIDRequired = "clientId is required for OIDC provider" - ErrUnsupportedStoreType = "unsupported store type: %s" - ErrUnsupportedProviderType = "unsupported provider type: %s" - ErrInvalidTokenFormat = "invalid session token format: %w" - ErrSessionValidationFailed = "session validation failed: %w" - ErrInvalidToken = "invalid token: %w" - ErrTokenNotValid = "token is not valid" - ErrInvalidTokenClaims = "invalid token claims" - ErrInvalidIssuer = "invalid issuer" - ErrMissingSessionID = "missing session ID" + ErrConfigCannotBeNil = "config cannot be nil" + ErrProviderCannotBeNil = "provider cannot be nil" + ErrProviderNameEmpty = "provider name cannot be empty" + ErrProviderTypeEmpty = "provider type cannot be empty" + ErrTokenCannotBeEmpty = "token cannot be empty" + ErrSessionTokenCannotBeEmpty = "session token cannot be empty" + ErrSessionIDCannotBeEmpty = "session ID cannot be empty" + ErrSTSServiceNotInitialized = "STS service not initialized" + ErrProviderNotInitialized = "provider not initialized" + ErrInvalidTokenDuration = "token duration must be positive" + ErrInvalidMaxSessionLength = "max session length must be positive" + ErrIssuerRequired = "issuer is required" + ErrSigningKeyTooShort = "signing key must be at least %d bytes" + ErrFilerAddressRequired = "filer address is required" + ErrClientIDRequired = "clientId is required for OIDC provider" + ErrUnsupportedStoreType = "unsupported store type: %s" + ErrUnsupportedProviderType = "unsupported provider type: %s" + ErrInvalidTokenFormat = "invalid session token format: %w" + ErrSessionValidationFailed = "session validation failed: %w" + ErrInvalidToken = "invalid token: %w" + ErrTokenNotValid = "token is not valid" + ErrInvalidTokenClaims = "invalid token claims" + ErrInvalidIssuer = "invalid issuer" + ErrMissingSessionID = "missing session ID" ) // JWT Claims @@ -97,11 +97,10 @@ const ( // AWS STS Actions const ( - ActionAssumeRole = "sts:AssumeRole" - ActionAssumeRoleWithWebIdentity = "sts:AssumeRoleWithWebIdentity" - ActionAssumeRoleWithCredentials = "sts:AssumeRoleWithCredentials" - ActionValidateSession = "sts:ValidateSession" - ActionRevokeSession = "sts:RevokeSession" + ActionAssumeRole = "sts:AssumeRole" + ActionAssumeRoleWithWebIdentity = "sts:AssumeRoleWithWebIdentity" + ActionAssumeRoleWithCredentials = "sts:AssumeRoleWithCredentials" + ActionValidateSession = "sts:ValidateSession" ) // Session File Prefixes @@ -122,17 +121,17 @@ const ( // Content Types const ( - ContentTypeJSON = "application/json" - ContentTypeFormURLEncoded = "application/x-www-form-urlencoded" + ContentTypeJSON = "application/json" + ContentTypeFormURLEncoded = "application/x-www-form-urlencoded" ) // Default Test Values const ( TestSigningKey32Chars = "test-signing-key-32-characters-long" - TestIssuer = "test-sts" - TestClientID = "test-client" - TestSessionID = "test-session-123" - TestValidToken = "valid_test_token" - TestInvalidToken = "invalid_token" - TestExpiredToken = "expired_token" + TestIssuer = "test-sts" + TestClientID = "test-client" + TestSessionID = "test-session-123" + TestValidToken = "valid_test_token" + TestInvalidToken = "invalid_token" + TestExpiredToken = "expired_token" ) diff --git a/weed/iam/sts/cross_instance_token_test.go b/weed/iam/sts/cross_instance_token_test.go index 285bd3885..713ba9ec1 100644 --- a/weed/iam/sts/cross_instance_token_test.go +++ b/weed/iam/sts/cross_instance_token_test.go @@ -144,12 +144,12 @@ func TestCrossInstanceTokenUsage(t *testing.T) { _, err = instanceB.ValidateSessionToken(ctx, sessionToken) require.NoError(t, err, "Token should be valid on Instance B initially") - // Attempt to revoke session on Instance C (in stateless system, this only validates) - err = instanceC.RevokeSession(ctx, sessionToken) + // Validate session on Instance C to verify cross-instance token compatibility + _, err = instanceC.ValidateSessionToken(ctx, sessionToken) require.NoError(t, err, "Instance C should be able to validate session token") - // In a stateless JWT system, tokens cannot be truly revoked without a shared blacklist - // The token should still be valid on all instances since it's self-contained + // In a stateless JWT system, tokens remain valid on all instances since they're self-contained + // No revocation is possible without breaking the stateless architecture _, err = instanceA.ValidateSessionToken(ctx, sessionToken) assert.NoError(t, err, "Token should still be valid on Instance A (stateless system)") diff --git a/weed/iam/sts/sts_service.go b/weed/iam/sts/sts_service.go index 2fb88eb45..5b9ca1e08 100644 --- a/weed/iam/sts/sts_service.go +++ b/weed/iam/sts/sts_service.go @@ -462,31 +462,18 @@ func (s *STSService) ValidateSessionToken(ctx context.Context, sessionToken stri return claims.ToSessionInfo(), nil } -// RevokeSession validates a session token (stateless operation) -// Note: In a stateless JWT system, sessions cannot be revoked without a blacklist. -// This method validates the token format but cannot actually revoke it. -// For production use, consider implementing a token blacklist or use short-lived tokens. -func (s *STSService) RevokeSession(ctx context.Context, sessionToken string) error { - if !s.initialized { - return fmt.Errorf("STS service not initialized") - } - - if sessionToken == "" { - return fmt.Errorf("session token cannot be empty") - } - - // Validate JWT token format - _, err := s.tokenGenerator.ValidateJWTWithClaims(sessionToken) - if err != nil { - return fmt.Errorf("invalid session token format: %w", err) - } - - // In a stateless system, we cannot revoke JWT tokens without a blacklist - // The token will naturally expire based on its embedded expiration time - glog.V(1).Infof("Session revocation requested for stateless token - token will expire naturally at its embedded expiration time") - - return nil -} +// NOTE: Session revocation is not supported in the stateless JWT design. +// +// In a stateless JWT system, tokens cannot be revoked without implementing a token blacklist, +// which would break the stateless architecture. Tokens remain valid until their natural +// expiration time. +// +// For applications requiring token revocation, consider: +// 1. Using shorter token lifespans (e.g., 15-30 minutes) +// 2. Implementing a distributed token blacklist (breaks stateless design) +// 3. Including a "jti" (JWT ID) claim for tracking specific tokens +// +// Use ValidateSessionToken() to verify if a token is valid and not expired. // Helper methods for AssumeRoleWithWebIdentity diff --git a/weed/iam/sts/sts_service_test.go b/weed/iam/sts/sts_service_test.go index 294885dca..a45536a16 100644 --- a/weed/iam/sts/sts_service_test.go +++ b/weed/iam/sts/sts_service_test.go @@ -265,8 +265,9 @@ func TestSessionTokenValidation(t *testing.T) { } } -// TestSessionRevocation tests session token revocation -func TestSessionRevocation(t *testing.T) { +// TestSessionTokenPersistence tests that JWT tokens remain valid throughout their lifetime +// Note: In the stateless JWT design, tokens cannot be revoked and remain valid until expiration +func TestSessionTokenPersistence(t *testing.T) { service := setupTestSTSService(t) ctx := context.Background() @@ -282,20 +283,18 @@ func TestSessionRevocation(t *testing.T) { sessionToken := response.Credentials.SessionToken - // Verify token is valid before revocation + // Verify token is valid initially session, err := service.ValidateSessionToken(ctx, sessionToken) assert.NoError(t, err) assert.NotNil(t, session) - - // Attempt to revoke the session (in stateless JWT system, this only validates the token) - err = service.RevokeSession(ctx, sessionToken) - assert.NoError(t, err, "RevokeSession should succeed in stateless system") - - // In a stateless JWT system, tokens cannot be truly revoked without a blacklist - // The token should still be valid since it's self-contained and hasn't expired - session, err = service.ValidateSessionToken(ctx, sessionToken) - assert.NoError(t, err, "Token should still be valid in stateless system") - assert.NotNil(t, session, "Session should be returned from JWT token") + assert.Equal(t, "test-session", session.SessionName) + + // In a stateless JWT system, tokens remain valid throughout their lifetime + // Multiple validations should all succeed as long as the token hasn't expired + session2, err := service.ValidateSessionToken(ctx, sessionToken) + assert.NoError(t, err, "Token should remain valid in stateless system") + assert.NotNil(t, session2, "Session should be returned from JWT token") + assert.Equal(t, session.SessionId, session2.SessionId, "Session ID should be consistent") } // Helper functions diff --git a/weed/iam/utils/arn_utils.go b/weed/iam/utils/arn_utils.go new file mode 100644 index 000000000..f47259b48 --- /dev/null +++ b/weed/iam/utils/arn_utils.go @@ -0,0 +1,28 @@ +package utils + +import "strings" + +// ExtractRoleNameFromPrincipal extracts role name from principal ARN +// Handles both STS assumed role and IAM role formats +func ExtractRoleNameFromPrincipal(principal string) string { + // Handle STS assumed role format: arn:seaweed:sts::assumed-role/RoleName/SessionName + stsPrefix := "arn:seaweed:sts::assumed-role/" + if strings.HasPrefix(principal, stsPrefix) { + remainder := principal[len(stsPrefix):] + // Split on first '/' to get role name + if slashIndex := strings.Index(remainder, "/"); slashIndex != -1 { + return remainder[:slashIndex] + } + // If no slash found, return the remainder (edge case) + return remainder + } + + // Handle IAM role format: arn:seaweed:iam::role/RoleName + iamPrefix := "arn:seaweed:iam::role/" + if strings.HasPrefix(principal, iamPrefix) { + return principal[len(iamPrefix):] + } + + // For backwards compatibility, return original principal if no recognized format + return principal +} diff --git a/weed/s3api/s3_iam_middleware.go b/weed/s3api/s3_iam_middleware.go index 8c42ca3a1..4454d538a 100644 --- a/weed/s3api/s3_iam_middleware.go +++ b/weed/s3api/s3_iam_middleware.go @@ -325,20 +325,6 @@ func extractSourceIP(r *http.Request) string { return r.RemoteAddr } -// extractRoleNameFromPrincipal extracts role name from assumed role principal ARN -func extractRoleNameFromPrincipal(principal string) string { - // Expected format: arn:seaweed:sts::assumed-role/RoleName/SessionName - prefix := "arn:seaweed:sts::assumed-role/" - if len(principal) > len(prefix) && principal[:len(prefix)] == prefix { - remainder := principal[len(prefix):] - // Split on first '/' to get role name - if slashIndex := strings.Index(remainder, "/"); slashIndex != -1 { - return remainder[:slashIndex] - } - } - return principal // Return original if parsing fails -} - // parseJWTToken parses a JWT token and returns its claims without verification // Note: This is for extracting claims only. Verification is done by the IAM system. func parseJWTToken(tokenString string) (jwt.MapClaims, error) { diff --git a/weed/s3api/s3_iam_simple_test.go b/weed/s3api/s3_iam_simple_test.go index 11db85c03..7bf403017 100644 --- a/weed/s3api/s3_iam_simple_test.go +++ b/weed/s3api/s3_iam_simple_test.go @@ -10,6 +10,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/iam/integration" "github.com/seaweedfs/seaweedfs/weed/iam/policy" "github.com/seaweedfs/seaweedfs/weed/iam/sts" + "github.com/seaweedfs/seaweedfs/weed/iam/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -212,7 +213,7 @@ func TestExtractRoleNameFromPrincipal(t *testing.T) { { name: "missing session name", principal: "arn:seaweed:sts::assumed-role/TestRole", - expected: "arn:seaweed:sts::assumed-role/TestRole", // Returns original on failure + expected: "TestRole", // Extracts role name even without session name }, { name: "empty principal", @@ -223,7 +224,7 @@ func TestExtractRoleNameFromPrincipal(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := extractRoleNameFromPrincipal(tt.principal) + result := utils.ExtractRoleNameFromPrincipal(tt.principal) assert.Equal(t, tt.expected, result) }) }