Browse Source

address comments

pull/7160/head
chrislu 2 months ago
parent
commit
d7314e932c
  1. 7
      test/s3/iam/Makefile
  2. 2
      test/s3/iam/docker-compose.yml
  3. 6
      test/s3/iam/s3_iam_distributed_test.go
  4. 37
      test/s3/iam/s3_keycloak_integration_test.go
  5. 29
      weed/iam/providers/provider.go
  6. 14
      weed/s3api/s3_iam_middleware.go

7
test/s3/iam/Makefile

@ -221,8 +221,11 @@ docker-up: ## Start all services with Docker Compose (including Keycloak)
@echo "🐳 Starting services with Docker Compose including Keycloak..."
@docker compose up -d
@echo "⏳ Waiting for services to be healthy..."
@sleep 30
@echo "✅ Services should 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)
@timeout 60 bash -c 'until curl -s http://localhost:8888 > /dev/null 2>&1; do sleep 2; done' || (echo "❌ Filer failed to become ready" && exit 1)
@timeout 60 bash -c 'until curl -s http://localhost:9333 > /dev/null 2>&1; do sleep 2; done' || (echo "❌ Master failed to become ready" && exit 1)
@echo "✅ All services are healthy and ready"
docker-down: ## Stop all Docker Compose services
@echo "🐳 Stopping Docker Compose services..."

2
test/s3/iam/docker-compose.yml

@ -107,8 +107,6 @@ services:
GLOG_v: "3"
command: >
sh -c "
echo 'Waiting for dependencies...' &&
sleep 10 &&
echo 'Starting S3 API with IAM...' &&
weed -v=3 s3 -ip=weed-s3 -port=8333
-filer=weed-filer:8888

6
test/s3/iam/s3_iam_distributed_test.go

@ -234,13 +234,13 @@ func TestS3IAMDistributedTests(t *testing.T) {
maxTotalErrorsRelaxed := 4 // Allow max 4 total errors (50% rate) - acceptable for resource-constrained CI
if len(errorList) > maxTotalErrorsRelaxed {
t.Errorf("❌ Too many total errors: %d (%.1f%%) - exceeds relaxed threshold of %d (%.1f%%). System is unstable under concurrent load.",
t.Errorf("❌ Too many total errors: %d (%.1f%%) - exceeds relaxed threshold of %d (%.1f%%). System is unstable under concurrent load.",
len(errorList), errorRate*100, maxTotalErrorsRelaxed, float64(maxTotalErrorsRelaxed)/float64(totalOperations)*100)
} else if len(errorList) > maxTotalErrorsStrict {
t.Logf("⚠️ Concurrent operations completed with %d errors (%.1f%%) - within relaxed CI limits. Normal CI environment variability.",
t.Logf("⚠️ Concurrent operations completed with %d errors (%.1f%%) - within relaxed CI limits. Normal CI environment variability.",
len(errorList), errorRate*100)
} else if len(errorList) > 0 {
t.Logf("✅ Concurrent operations completed with %d errors (%.1f%%) - good performance for CI environment!",
t.Logf("✅ Concurrent operations completed with %d errors (%.1f%%) - good performance for CI environment!",
len(errorList), errorRate*100)
} else {
t.Logf("🎉 All %d concurrent operations completed successfully - excellent CI performance!", totalOperations)

37
test/s3/iam/s3_keycloak_integration_test.go

@ -78,11 +78,8 @@ func TestKeycloakAuthentication(t *testing.T) {
parts := strings.Split(token, ".")
if len(parts) >= 2 {
payload := parts[1]
// Add padding if needed
for len(payload)%4 != 0 {
payload += "="
}
decoded, err := base64.StdEncoding.DecodeString(payload)
// JWTs use URL-safe base64 encoding without padding (RFC 4648 §5)
decoded, err := base64.RawURLEncoding.DecodeString(payload)
if err == nil {
var claims map[string]interface{}
if json.Unmarshal(decoded, &claims) == nil {
@ -92,24 +89,24 @@ func TestKeycloakAuthentication(t *testing.T) {
}
}
// First test with direct HTTP request to verify OIDC authentication works
t.Logf("Testing with direct HTTP request...")
err = framework.TestKeycloakTokenDirectly(token)
require.NoError(t, err, "Direct HTTP test should succeed")
// First test with direct HTTP request to verify OIDC authentication works
t.Logf("Testing with direct HTTP request...")
err = framework.TestKeycloakTokenDirectly(token)
require.NoError(t, err, "Direct HTTP test should succeed")
// Create S3 client with Keycloak token
s3Client, err := framework.CreateS3ClientWithKeycloakToken(token)
require.NoError(t, err)
// Create S3 client with Keycloak token
s3Client, err := framework.CreateS3ClientWithKeycloakToken(token)
require.NoError(t, err)
// Test that read-only user can list buckets
t.Logf("Testing ListBuckets with AWS SDK...")
_, err = s3Client.ListBuckets(&s3.ListBucketsInput{})
assert.NoError(t, err, "Read-only user should be able to list buckets")
// Test that read-only user can list buckets
t.Logf("Testing ListBuckets with AWS SDK...")
_, err = s3Client.ListBuckets(&s3.ListBucketsInput{})
assert.NoError(t, err, "Read-only user should be able to list buckets")
// Test that read-only user cannot create buckets
t.Logf("Testing CreateBucket with AWS SDK...")
err = framework.CreateBucket(s3Client, testKeycloakBucket+"-readonly")
assert.Error(t, err, "Read-only user should not be able to create buckets")
// Test that read-only user cannot create buckets
t.Logf("Testing CreateBucket with AWS SDK...")
err = framework.CreateBucket(s3Client, testKeycloakBucket+"-readonly")
assert.Error(t, err, "Read-only user should not be able to create buckets")
})
t.Run("invalid_user_authentication", func(t *testing.T) {

29
weed/iam/providers/provider.go

@ -4,7 +4,8 @@ import (
"context"
"fmt"
"net/mail"
"path/filepath"
"regexp"
"strings"
"time"
"github.com/seaweedfs/seaweedfs/weed/glog"
@ -219,12 +220,28 @@ func (r *MappingRule) Matches(claims *TokenClaims) bool {
}
// matchValue checks if a value matches the rule value (with wildcard support)
// Uses AWS IAM-compliant case-insensitive wildcard matching for consistency with policy engine
func (r *MappingRule) matchValue(value string) bool {
matched, err := filepath.Match(r.Value, value)
matched := awsWildcardMatch(r.Value, value)
glog.V(3).Infof("AWS IAM pattern match result: '%s' matches '%s' = %t", value, r.Value, matched)
return matched
}
// awsWildcardMatch performs case-insensitive wildcard matching like AWS IAM
// This function ensures consistent matching behavior across the IAM system
func awsWildcardMatch(pattern, value string) bool {
// Convert pattern to regex with case-insensitive matching
// AWS uses * for any sequence and ? for any single character
regexPattern := strings.ReplaceAll(pattern, "*", ".*")
regexPattern = strings.ReplaceAll(regexPattern, "?", ".")
regexPattern = "^" + regexPattern + "$"
// Compile with case-insensitive flag
regex, err := regexp.Compile("(?i)" + regexPattern)
if err != nil {
glog.V(3).Infof("Error matching '%s' against pattern '%s': %v", value, r.Value, err)
return false
// Fallback to simple case-insensitive comparison if regex fails
return strings.EqualFold(pattern, value)
}
glog.V(3).Infof("Pattern match result: '%s' matches '%s' = %t", value, r.Value, matched)
return matched
return regex.MatchString(value)
}

14
weed/s3api/s3_iam_middleware.go

@ -285,7 +285,7 @@ func determineGranularS3Action(r *http.Request, fallbackAction Action, bucket st
hasGranularIndicators := hasSpecificQueryParameters(query)
// Only check for method-action mismatch when there are NO granular indicators
// This handles test scenarios where method and action don't align but no specific query params exist
// This provides fallback behavior for cases where HTTP method doesn't align with intended action
if !hasGranularIndicators && isMethodActionMismatch(method, fallbackAction) {
return mapLegacyActionToIAM(fallbackAction)
}
@ -482,28 +482,28 @@ func hasSpecificQueryParameters(query url.Values) bool {
return false
}
// isMethodActionMismatch detects when HTTP method doesn't align with intended action
// This handles testing scenarios and edge cases where method and action don't naturally correspond
// isMethodActionMismatch detects when HTTP method doesn't align with the intended S3 action
// This provides a mechanism to use fallback action mapping when there's a semantic mismatch
func isMethodActionMismatch(method string, fallbackAction Action) bool {
switch fallbackAction {
case s3_constants.ACTION_WRITE:
// WRITE actions should typically use PUT, POST, or DELETE methods
// If method is GET/HEAD, it's likely a test or edge case - use fallback
// GET/HEAD methods indicate read-oriented operations
return method == "GET" || method == "HEAD"
case s3_constants.ACTION_READ:
// READ actions should typically use GET or HEAD methods
// If method is PUT, POST, DELETE, it's likely a test or edge case - use fallback
// PUT, POST, DELETE methods indicate write-oriented operations
return method == "PUT" || method == "POST" || method == "DELETE"
case s3_constants.ACTION_LIST:
// LIST actions should typically use GET method
// If method is PUT, POST, DELETE, it's likely a test or edge case - use fallback
// PUT, POST, DELETE methods indicate write-oriented operations
return method == "PUT" || method == "POST" || method == "DELETE"
case s3_constants.ACTION_DELETE_BUCKET:
// DELETE_BUCKET should use DELETE method
// If method is GET, HEAD, PUT, POST, it's likely a test or edge case - use fallback
// Other methods indicate different operation types
return method != "DELETE"
default:

Loading…
Cancel
Save