Fix STS authorization in streaming/chunked uploads (#7988)

* Fix STS authorization in streaming/chunked uploads During streaming/chunked uploads (SigV4 streaming), authorization happens twice: 1. Initial authorization in authRequestWithAuthType() - works correctly 2. Second authorization in verifyV4Signature() - was failing for STS The issue was that verifyV4Signature() only used identity.canDo() for permission checks, which always denies STS identities (they have empty Actions). This bypassed IAM authorization completely. This commit makes verifyV4Signature() IAM-aware by adding the same fallback logic used in authRequestWithAuthType(): - Traditional identities (with Actions) use legacy canDo() check - STS/JWT identities (empty Actions) fall back to IAM authorization Fixes: https://github.com/seaweedfs/seaweedfs/pull/7986#issuecomment-3723196038 * Add comprehensive unit tests for STS authorization in streaming uploads Created test suite to verify that verifyV4Signature properly handles STS identities by falling back to IAM authorization when shouldCheckPermissions is true. Tests cover: - STS identities with IAM integration (allow and deny cases) - STS identities without IAM integration (should deny) - Traditional identities with Actions (canDo check) - Permission check bypass when shouldCheckPermissions=false - Specific streaming upload scenario from bug report - Action determination based on HTTP method All tests pass successfully. * Refactor authorization logic to avoid duplication Centralized the authorization logic into IdentityAccessManagement.VerifyActionPermission. Updated auth_signature_v4.go and auth_credentials.go to use this new helper. Updated tests to clarify that they mirror the centralized logic. * Refactor tests to use VerifyActionPermission directly Introduced IAMIntegration interface to facilitate mocking of internal IAM integration logic. Updated IdentityAccessManagement to use the interface. Updated tests to directy call VerifyActionPermission using a mocked IAM integration, eliminating duplicated logic in tests. * fix(s3api): ensure static config file takes precedence and refactor tests - Track if configuration was loaded from a static file using `useStaticConfig`. - Ignore filer-based IAM updates when a static configuration is in use to respect "Highest Priority" rule. - Refactor `TestVerifyV4SignatureWithSTSIdentity` to use `VerifyActionPermission` directly. - Fix typed nil interface panic in authorization test.
3 days ago · abfa64456b
4 changed files with 310 additions and 20 deletions
--- a/weed/s3api/auth_credentials.go
+++ b/weed/s3api/auth_credentials.go
@ -55,7 +55,7 @@ type IdentityAccessManagement struct {
 	grpcDialOption    grpc.DialOption

 	// IAM Integration for advanced features
-	iamIntegration *S3IAMIntegration
+	iamIntegration IAMIntegration

 	// Bucket policy engine for evaluating bucket policies
 	policyEngine *BucketPolicyEngine
@ -702,18 +702,9 @@ func (iam *IdentityAccessManagement) authRequestWithAuthType(r *http.Request, ac

 		// Only check IAM if bucket policy didn't explicitly allow
 		if !policyAllows {
-			// Traditional identities (with Actions from -s3.config) use legacy auth,
-			// JWT/STS identities (no Actions) use IAM authorization
-			if len(identity.Actions) > 0 {
-				if !identity.canDo(action, bucket, object) {
-					return identity, s3err.ErrAccessDenied, reqAuthType
-				}
-			} else if iam.iamIntegration != nil {
-				if errCode := iam.authorizeWithIAM(r, identity, action, bucket, object); errCode != s3err.ErrNone {
-					return identity, errCode, reqAuthType
-				}
-			} else {
-				return identity, s3err.ErrAccessDenied, reqAuthType
+			// Use centralized permission check
+			if errCode := iam.VerifyActionPermission(r, identity, action, bucket, object); errCode != s3err.ErrNone {
+				return identity, errCode, reqAuthType
 			}
 		}
 	}
@ -990,6 +981,23 @@ func determineIAMAuthPath(sessionToken, principal, principalArn string) iamAuthP
 	return iamAuthPathNone
 }

+// VerifyActionPermission checks if the identity is allowed to perform the action on the resource.
+// It handles both traditional identities (via Actions) and IAM/STS identities (via Policy).
+func (iam *IdentityAccessManagement) VerifyActionPermission(r *http.Request, identity *Identity, action Action, bucket, object string) s3err.ErrorCode {
+	// Traditional identities (with Actions from -s3.config) use legacy auth,
+	// JWT/STS identities (no Actions) use IAM authorization
+	if len(identity.Actions) > 0 {
+		if !identity.canDo(action, bucket, object) {
+			return s3err.ErrAccessDenied
+		}
+		return s3err.ErrNone
+	} else if iam.iamIntegration != nil {
+		return iam.authorizeWithIAM(r, identity, action, bucket, object)
+	}
+
+	return s3err.ErrAccessDenied
+}
+
 // authorizeWithIAM authorizes requests using the IAM integration policy engine
 func (iam *IdentityAccessManagement) authorizeWithIAM(r *http.Request, identity *Identity, action Action, bucket string, object string) s3err.ErrorCode {
 	ctx := r.Context()
--- a/weed/s3api/auth_signature_v4.go
+++ b/weed/s3api/auth_signature_v4.go
@ -248,8 +248,10 @@ func (iam *IdentityAccessManagement) verifyV4Signature(r *http.Request, shouldCh
 		if r.Method != http.MethodGet && r.Method != http.MethodHead {
 			action = s3_constants.ACTION_WRITE
 		}
-		if !identity.canDo(Action(action), bucket, object) {
-			return nil, nil, "", nil, s3err.ErrAccessDenied
+
+		// Use centralized permission check
+		if errCode = iam.VerifyActionPermission(r, identity, Action(action), bucket, object); errCode != s3err.ErrNone {
+			return nil, nil, "", nil, errCode
 		}
 	}

@ -301,15 +303,15 @@ func (iam *IdentityAccessManagement) verifyV4Signature(r *http.Request, shouldCh

 // validateSTSSessionToken validates an STS session token and extracts temporary credentials
 func (iam *IdentityAccessManagement) validateSTSSessionToken(r *http.Request, sessionToken string, accessKey string) (*Identity, *Credential, s3err.ErrorCode) {
-	// Check if IAM integration with STS is available
-	if iam.iamIntegration == nil || iam.iamIntegration.stsService == nil {
-		glog.V(2).Infof("STS service not available, cannot validate session token")
+	// Check if IAM integration is available
+	if iam.iamIntegration == nil {
+		glog.V(2).Infof("IAM integration not available, cannot validate session token")
 		return nil, nil, s3err.ErrInvalidAccessKeyID
 	}

 	// Validate the session token with the STS service
 	ctx := r.Context()
-	sessionInfo, err := iam.iamIntegration.stsService.ValidateSessionToken(ctx, sessionToken)
+	sessionInfo, err := iam.iamIntegration.ValidateSessionToken(ctx, sessionToken)
 	if err != nil {
 		glog.V(2).Infof("Failed to validate STS session token: %v", err)
 		return nil, nil, s3err.ErrInvalidAccessKeyID
--- a/weed/s3api/auth_signature_v4_sts_test.go
+++ b/weed/s3api/auth_signature_v4_sts_test.go
@ -0,0 +1,265 @@
+package s3api
+
+import (
+	"context"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/seaweedfs/seaweedfs/weed/iam/sts"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
+)
+
+// MockIAMIntegration is a mock implementation of IAM integration for testing
+type MockIAMIntegration struct {
+	authorizeFunc func(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode
+}
+
+func (m *MockIAMIntegration) AuthorizeAction(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode {
+	if m.authorizeFunc != nil {
+		return m.authorizeFunc(ctx, identity, action, bucket, object, r)
+	}
+	return s3err.ErrNone
+}
+
+func (m *MockIAMIntegration) AuthenticateJWT(ctx context.Context, r *http.Request) (*IAMIdentity, s3err.ErrorCode) {
+	return nil, s3err.ErrNotImplemented
+}
+
+func (m *MockIAMIntegration) ValidateSessionToken(ctx context.Context, token string) (*sts.SessionInfo, error) {
+	return nil, nil // Not needed for these tests
+}
+
+// TestVerifyV4SignatureWithSTSIdentity tests that verifyV4Signature properly handles STS identities
+// by falling back to IAM authorization when shouldCheckPermissions is true
+func TestVerifyV4SignatureWithSTSIdentity(t *testing.T) {
+	tests := []struct {
+		name                   string
+		identity               *Identity
+		shouldCheckPermissions bool
+		iamIntegration         *MockIAMIntegration
+		expectedError          s3err.ErrorCode
+		description            string
+	}{
+		{
+			name: "STS identity with IAM integration - should authorize via IAM",
+			identity: &Identity{
+				Name:         "arn:aws:sts::assumed-role/adminRole/s3-session",
+				Account:      &AccountAdmin,
+				Actions:      []Action{}, // Empty actions = STS identity
+				PrincipalArn: "arn:aws:sts::assumed-role/adminRole/s3-session",
+			},
+			shouldCheckPermissions: true,
+			iamIntegration: &MockIAMIntegration{
+				authorizeFunc: func(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode {
+					// Simulate successful IAM authorization
+					return s3err.ErrNone
+				},
+			},
+			expectedError: s3err.ErrNone,
+			description:   "STS identity should be authorized via IAM when Actions is empty",
+		},
+		{
+			name: "STS identity with IAM integration - IAM denies",
+			identity: &Identity{
+				Name:         "arn:aws:sts::assumed-role/readOnlyRole/s3-session",
+				Account:      &AccountAdmin,
+				Actions:      []Action{}, // Empty actions = STS identity
+				PrincipalArn: "arn:aws:sts::assumed-role/readOnlyRole/s3-session",
+			},
+			shouldCheckPermissions: true,
+			iamIntegration: &MockIAMIntegration{
+				authorizeFunc: func(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode {
+					// Simulate IAM denying access (e.g., read-only role trying to write)
+					if action == s3_constants.ACTION_WRITE {
+						return s3err.ErrAccessDenied
+					}
+					return s3err.ErrNone
+				},
+			},
+			expectedError: s3err.ErrAccessDenied,
+			description:   "STS identity should be denied when IAM denies access",
+		},
+		{
+			name: "STS identity without IAM integration - should deny",
+			identity: &Identity{
+				Name:         "arn:aws:sts::assumed-role/adminRole/s3-session",
+				Account:      &AccountAdmin,
+				Actions:      []Action{}, // Empty actions = STS identity
+				PrincipalArn: "arn:aws:sts::assumed-role/adminRole/s3-session",
+			},
+			shouldCheckPermissions: true,
+			iamIntegration:         nil, // No IAM integration
+			expectedError:          s3err.ErrAccessDenied,
+			description:            "STS identity should be denied when no IAM integration is available",
+		},
+		{
+			name: "Traditional identity with Actions - should use canDo",
+			identity: &Identity{
+				Name:    "traditional-user",
+				Account: &AccountAdmin,
+				Actions: []Action{s3_constants.ACTION_WRITE}, // Has actions = traditional identity
+			},
+			shouldCheckPermissions: true,
+			iamIntegration:         nil, // IAM integration not needed for traditional identities
+			expectedError:          s3err.ErrNone,
+			description:            "Traditional identity with Actions should use canDo check",
+		},
+		{
+			name: "Traditional identity with Actions - canDo denies",
+			identity: &Identity{
+				Name:    "read-only-user",
+				Account: &AccountAdmin,
+				Actions: []Action{s3_constants.ACTION_READ}, // Only has READ action
+			},
+			shouldCheckPermissions: true,
+			iamIntegration:         nil,
+			expectedError:          s3err.ErrAccessDenied,
+			description:            "Traditional identity should be denied when canDo fails (PUT requires WRITE)",
+		},
+		{
+			name: "shouldCheckPermissions false - skip authorization",
+			identity: &Identity{
+				Name:         "arn:aws:sts::assumed-role/adminRole/s3-session",
+				Account:      &AccountAdmin,
+				Actions:      []Action{}, // Empty actions = STS identity
+				PrincipalArn: "arn:aws:sts::assumed-role/adminRole/s3-session",
+			},
+			shouldCheckPermissions: false, // Skip permission check
+			iamIntegration:         nil,
+			expectedError:          s3err.ErrNone,
+			description:            "When shouldCheckPermissions is false, authorization should be skipped",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create a mock request for PUT operation (requires WRITE action)
+			req, err := http.NewRequest("PUT", "http://s3.amazonaws.com/test-bucket/test-object", nil)
+			require.NoError(t, err)
+			req.Header.Set("Host", "s3.amazonaws.com")
+
+			// Mock the permission check logic from verifyV4Signature (now centralized in VerifyActionPermission)
+			var errCode s3err.ErrorCode
+			if tt.shouldCheckPermissions {
+				bucket, object := s3_constants.GetBucketAndObject(req)
+				action := s3_constants.ACTION_READ
+				if req.Method != http.MethodGet && req.Method != http.MethodHead {
+					action = s3_constants.ACTION_WRITE
+				}
+
+				// Create minimal IAM instance with mock integration
+				var integration IAMIntegration
+				if tt.iamIntegration != nil {
+					integration = tt.iamIntegration
+				}
+				iam := &IdentityAccessManagement{
+					iamIntegration: integration,
+				}
+				errCode = iam.VerifyActionPermission(req, tt.identity, Action(action), bucket, object)
+			}
+
+			// Verify the result
+			assert.Equal(t, tt.expectedError, errCode, tt.description)
+
+			// Additional verification for STS identities
+			if len(tt.identity.Actions) == 0 && tt.shouldCheckPermissions {
+				if tt.iamIntegration != nil {
+					// When IAM integration exists, it should have been called
+					// The result depends on what the mock returns
+				} else {
+					assert.Equal(t, s3err.ErrAccessDenied, errCode, "STS identity should be denied without IAM integration")
+				}
+			}
+		})
+	}
+}
+
+// TestVerifyV4SignatureSTSStreamingUpload tests the specific scenario from the bug report:
+// STS identities in streaming/chunked uploads should be authorized via IAM
+func TestVerifyV4SignatureSTSStreamingUpload(t *testing.T) {
+	// Create an STS identity (empty Actions)
+	stsIdentity := &Identity{
+		Name:         "arn:aws:sts::assumed-role/adminRole/s3-session",
+		Account:      &AccountAdmin,
+		Actions:      []Action{}, // Empty - this is an STS identity
+		PrincipalArn: "arn:aws:sts::assumed-role/adminRole/s3-session",
+	}
+
+	// Track whether IAM authorization was called
+	iamAuthCalled := false
+
+	// Create IAM integration mock that allows the action
+	iamMock := &MockIAMIntegration{
+		authorizeFunc: func(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode {
+			iamAuthCalled = true
+			// Verify we're checking the right identity
+			assert.Equal(t, "arn:aws:sts::assumed-role/adminRole/s3-session", identity.Name)
+			assert.Equal(t, s3_constants.ACTION_WRITE, string(action))
+			assert.Equal(t, "test-bucket", bucket)
+			assert.Equal(t, "test-object", object)
+			return s3err.ErrNone
+		},
+	}
+
+	// Create a streaming upload request (PUT with streaming content)
+	req, err := http.NewRequest("PUT", "/test-bucket/test-object", nil)
+	require.NoError(t, err)
+	req.Host = "s3.amazonaws.com"
+	req.Header.Set("Host", "s3.amazonaws.com")
+	req.Header.Set("x-amz-content-sha256", "STREAMING-AWS4-HMAC-SHA256-PAYLOAD")
+	req.Header.Set("Content-Encoding", "aws-chunked")
+	req.Header.Set("X-Amz-Security-Token", "test-session-token")
+
+	// Simulate the permission check with shouldCheckPermissions=true
+	// This is what happens in calculateSeedSignature -> verifyV4Signature(r, true)
+	bucket := "test-bucket"
+	object := "test-object"
+	action := s3_constants.ACTION_WRITE
+
+	var errCode s3err.ErrorCode
+
+	// Create minimal IAM instance logic
+	iam := &IdentityAccessManagement{
+		iamIntegration: iamMock,
+	}
+
+	errCode = iam.VerifyActionPermission(req, stsIdentity, Action(action), bucket, object)
+
+	// Verify that the STS identity is authorized via IAM
+	assert.Equal(t, s3err.ErrNone, errCode, "STS identity should be authorized via IAM for streaming upload")
+	assert.True(t, iamAuthCalled, "IAM authorization should have been called for STS identity")
+}
+
+// TestVerifyV4SignatureActionDetermination tests that the correct action is determined
+// based on the HTTP method
+func TestVerifyV4SignatureActionDetermination(t *testing.T) {
+	tests := []struct {
+		method         string
+		expectedAction string
+	}{
+		{http.MethodGet, s3_constants.ACTION_READ},
+		{http.MethodHead, s3_constants.ACTION_READ},
+		{http.MethodPut, s3_constants.ACTION_WRITE},
+		{http.MethodPost, s3_constants.ACTION_WRITE},
+		{http.MethodDelete, s3_constants.ACTION_WRITE},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.method, func(t *testing.T) {
+			req, err := http.NewRequest(tt.method, "http://s3.amazonaws.com/bucket/object", nil)
+			require.NoError(t, err)
+
+			// Determine action the same way verifyV4Signature does
+			action := s3_constants.ACTION_READ
+			if req.Method != http.MethodGet && req.Method != http.MethodHead {
+				action = s3_constants.ACTION_WRITE
+			}
+
+			assert.Equal(t, tt.expectedAction, action, "Action should match expected for method %s", tt.method)
+		})
+	}
+}
--- a/weed/s3api/s3_iam_middleware.go
+++ b/weed/s3api/s3_iam_middleware.go
@ -18,6 +18,13 @@ import (
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
 )

+// IAMIntegration defines the interface for IAM integration
+type IAMIntegration interface {
+	AuthenticateJWT(ctx context.Context, r *http.Request) (*IAMIdentity, s3err.ErrorCode)
+	AuthorizeAction(ctx context.Context, identity *IAMIdentity, action Action, bucket string, objectKey string, r *http.Request) s3err.ErrorCode
+	ValidateSessionToken(ctx context.Context, token string) (*sts.SessionInfo, error)
+}
+
 // S3IAMIntegration provides IAM integration for S3 API
 type S3IAMIntegration struct {
 	iamManager   *integration.IAMManager
@ -167,6 +174,14 @@ func (s3iam *S3IAMIntegration) AuthenticateJWT(ctx context.Context, r *http.Requ
 	return identity, s3err.ErrNone
 }

+// ValidateSessionToken checks the validity of an STS session token
+func (s3iam *S3IAMIntegration) ValidateSessionToken(ctx context.Context, token string) (*sts.SessionInfo, error) {
+	if s3iam.stsService == nil {
+		return nil, fmt.Errorf("STS service not available")
+	}
+	return s3iam.stsService.ValidateSessionToken(ctx, token)
+}
+
 // AuthorizeAction authorizes actions using our policy engine
 func (s3iam *S3IAMIntegration) AuthorizeAction(ctx context.Context, identity *IAMIdentity, action Action, bucket string, objectKey string, r *http.Request) s3err.ErrorCode {
 	if !s3iam.enabled {
@ -463,7 +478,7 @@ func (s3a *S3ApiServer) SetIAMIntegration(iamManager *integration.IAMManager) {
 // EnhancedS3ApiServer extends S3ApiServer with IAM integration
 type EnhancedS3ApiServer struct {
 	*S3ApiServer
-	iamIntegration *S3IAMIntegration
+	iamIntegration IAMIntegration
 }

 // NewEnhancedS3ApiServer creates an S3 API server with IAM integration