refactor

weed/s3api/s3api_object_handlers_delete.go

@@ -53,8 +53,8 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 		// Handle versioned delete
 		if versionId != "" {
 			// Check object lock permissions before deleting specific version
-			bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object)
-			if err := s3a.checkObjectLockPermissions(r, bucket, object, versionId, bypassGovernance); err != nil {
+			governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
+			if err := s3a.enforceObjectLockProtections(r, bucket, object, versionId, governanceBypassAllowed); err != nil {
 				glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
 				s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
 				return
@@ -73,8 +73,8 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 		} else {
 			// Check object lock permissions before creating delete marker
 			// AWS S3 behavior: delete operations fail if latest version has retention protection
-			bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object)
-			if err := s3a.checkObjectLockPermissions(r, bucket, object, "", bypassGovernance); err != nil {
+			governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
+			if err := s3a.enforceObjectLockProtections(r, bucket, object, "", governanceBypassAllowed); err != nil {
 				glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
 				s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
 				return
@@ -95,8 +95,8 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 	} else {
 		// Handle regular delete (non-versioned)
 		// Check object lock permissions before deleting object
-		bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object)
-		if err := s3a.checkObjectLockPermissions(r, bucket, object, "", bypassGovernance); err != nil {
+		governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
+		if err := s3a.enforceObjectLockProtections(r, bucket, object, "", governanceBypassAllowed); err != nil {
 			glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
 			s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
 			return
@@ -231,8 +231,8 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
 			// Check object lock permissions before deletion (only for versioned buckets)
 			if versioningEnabled {
 				// Validate governance bypass for this specific object
-				bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object.Key)
-				if err := s3a.checkObjectLockPermissions(r, bucket, object.Key, object.VersionId, bypassGovernance); err != nil {
+				governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key)
+				if err := s3a.enforceObjectLockProtections(r, bucket, object.Key, object.VersionId, governanceBypassAllowed); err != nil {
 					glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object.Key, object.VersionId, err)
 					deleteErrors = append(deleteErrors, DeleteError{
 						Code:      s3err.GetAPIError(s3err.ErrAccessDenied).Code,

weed/s3api/s3api_object_handlers_put.go

@@ -119,8 +119,8 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request)
 		// For non-versioned buckets, check if existing object has object lock protections
 		// that would prevent overwrite (PUT operations overwrite existing objects in non-versioned buckets)
 		if !versioningEnabled {
-			bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object)
-			if err := s3a.checkObjectLockPermissions(r, bucket, object, "", bypassGovernance); err != nil {
+			governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
+			if err := s3a.enforceObjectLockProtections(r, bucket, object, "", governanceBypassAllowed); err != nil {
 				glog.V(2).Infof("PutObjectHandler: object lock permissions check failed for %s/%s: %v", bucket, object, err)
 				s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
 				return

weed/s3api/s3api_object_handlers_retention.go

@@ -25,8 +25,8 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http
 	// Get version ID from query parameters
 	versionId := r.URL.Query().Get("versionId")
 
-	// Validate governance bypass permission
-	bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object)
+	// Evaluate governance bypass request (header + permission validation)
+	governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
 
 	// Parse retention configuration from request body
 	retention, err := parseObjectRetention(r)
@@ -44,7 +44,7 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http
 	}
 
 	// Set retention on the object
-	if err := s3a.setObjectRetention(bucket, object, versionId, retention, bypassGovernance); err != nil {
+	if err := s3a.setObjectRetention(bucket, object, versionId, retention, governanceBypassAllowed); err != nil {
 		glog.Errorf("PutObjectRetentionHandler: failed to set retention: %v", err)
 
 		// Handle specific error cases

weed/s3api/s3api_object_retention.go

@@ -596,7 +596,14 @@ func (s3a *S3ApiServer) getLegalHoldFromEntry(entry *filer_pb.Entry) (*ObjectLeg
 	return legalHold, isActive, nil
 }
 
-// checkGovernanceBypassPermission checks if the user has permission to bypass governance retention
+// checkGovernanceBypassPermission validates if the user has IAM permission to bypass governance retention.
+// This is the low-level permission check that integrates with the IAM system.
+//
+// Returns true if:
+// - User has s3:BypassGovernanceRetention permission for the resource, OR
+// - User has Admin permissions for the resource
+//
+// This function does NOT check if the bypass header is present - that's handled separately.
 func (s3a *S3ApiServer) checkGovernanceBypassPermission(request *http.Request, bucket, object string) bool {
 	// Use the existing IAM auth system to check the specific permission
 	// Create the governance bypass action with proper bucket/object concatenation
@@ -627,20 +634,47 @@ func (s3a *S3ApiServer) checkGovernanceBypassPermission(request *http.Request, b
 	return false
 }
 
-// validateGovernanceBypass checks if the user has requested governance bypass via header
-// and validates they have the required s3:BypassGovernanceRetention permission.
-// This helper method consolidates the repetitive governance bypass validation logic
-// used across multiple handlers (DELETE, PUT, etc.).
-func (s3a *S3ApiServer) validateGovernanceBypass(r *http.Request, bucket, object string) bool {
-	// Check if governance bypass header is present
+// evaluateGovernanceBypassRequest determines if a governance bypass should be allowed.
+// This is the high-level validation that combines header checking with permission validation.
+//
+// AWS S3 requires BOTH conditions:
+// 1. Client sends x-amz-bypass-governance-retention: true header (intent)
+// 2. User has s3:BypassGovernanceRetention IAM permission (authorization)
+//
+// Returns true only if both conditions are met.
+// Used by all handlers that need to check governance bypass (DELETE, PUT, etc.).
+func (s3a *S3ApiServer) evaluateGovernanceBypassRequest(r *http.Request, bucket, object string) bool {
+	// Step 1: Check if governance bypass was requested via header
 	bypassRequested := r.Header.Get("x-amz-bypass-governance-retention") == "true"
+	if !bypassRequested {
+		// No bypass requested - normal retention enforcement applies
+		return false
+	}
 
-	// Only allow bypass if both header is present AND user has permission
-	return bypassRequested && s3a.checkGovernanceBypassPermission(r, bucket, object)
+	// Step 2: Validate user has permission to bypass governance retention
+	hasPermission := s3a.checkGovernanceBypassPermission(r, bucket, object)
+	if !hasPermission {
+		glog.V(2).Infof("Governance bypass denied for %s/%s: user lacks s3:BypassGovernanceRetention permission", bucket, object)
+		return false
+	}
+
+	glog.V(2).Infof("Governance bypass granted for %s/%s: header present and user has permission", bucket, object)
+	return true
 }
 
-// checkObjectLockPermissions checks if an object can be deleted or modified
-func (s3a *S3ApiServer) checkObjectLockPermissions(request *http.Request, bucket, object, versionId string, bypassGovernance bool) error {
+// enforceObjectLockProtections checks if an object operation should be blocked by object lock.
+// This function enforces retention and legal hold policies based on pre-validated permissions.
+//
+// Parameters:
+//   - request: HTTP request (for logging/context only - permissions already validated)
+//   - bucket, object, versionId: Object identifier
+//   - governanceBypassAllowed: Pre-validated governance bypass permission (from evaluateGovernanceBypassRequest)
+//
+// Important: The governanceBypassAllowed parameter is TRUSTED - it should only be set to true
+// if evaluateGovernanceBypassRequest() has already validated both header presence and IAM permissions.
+//
+// Returns error if operation should be blocked, nil if operation is allowed.
+func (s3a *S3ApiServer) enforceObjectLockProtections(request *http.Request, bucket, object, versionId string, governanceBypassAllowed bool) error {
 	// Get the object entry to check both retention and legal hold
 	// For delete operations without versionId, we need to check the latest version
 	var entry *filer_pb.Entry
@@ -691,10 +725,10 @@ func (s3a *S3ApiServer) checkObjectLockPermissions(request *http.Request, bucket
 		}
 
 		if retention.Mode == s3_constants.RetentionModeGovernance {
-			if !bypassGovernance {
+			if !governanceBypassAllowed {
 				return ErrGovernanceModeActive
 			}
-			// Note: bypassGovernance parameter is already validated by validateGovernanceBypass()
+			// Note: governanceBypassAllowed parameter is already validated by evaluateGovernanceBypassRequest()
 			// which checks both header presence and IAM permissions, so we trust it here
 		}
 	}