only checkObjectLockPermissions if versioningEnabled

weed/s3api/s3api_object_handlers_delete.go

@@ -49,12 +49,14 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 		auditLog = s3err.GetAccessLog(r, http.StatusNoContent, s3err.ErrNone)
 	}
 
-	// Check object lock permissions before deletion
-	bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
-	if err := s3a.checkObjectLockPermissions(bucket, object, versionId, bypassGovernance); 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
+	// Check object lock permissions before deletion (only for versioned buckets)
+	if versioningEnabled {
+		bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
+		if err := s3a.checkObjectLockPermissions(bucket, object, versionId, bypassGovernance); 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
+		}
 	}
 
 	if versioningEnabled {
@@ -192,6 +194,18 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
 	// Check for bypass governance retention header
 	bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
 
+	// Check if versioning is enabled for the bucket (needed for object lock checks)
+	versioningEnabled, err := s3a.isVersioningEnabled(bucket)
+	if err != nil {
+		if err == filer_pb.ErrNotFound {
+			s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
+			return
+		}
+		glog.Errorf("Error checking versioning status for bucket %s: %v", bucket, err)
+		s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)
+		return
+	}
+
 	s3a.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
 
 		// delete file entries
@@ -200,15 +214,17 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
 				continue
 			}
 
-			// Check object lock permissions before deletion
-			if err := s3a.checkObjectLockPermissions(bucket, object.ObjectName, "", bypassGovernance); err != nil {
-				glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s: %v", bucket, object.ObjectName, err)
-				deleteErrors = append(deleteErrors, DeleteError{
-					Code:    "AccessDenied",
-					Message: err.Error(),
-					Key:     object.ObjectName,
-				})
-				continue
+			// Check object lock permissions before deletion (only for versioned buckets)
+			if versioningEnabled {
+				if err := s3a.checkObjectLockPermissions(bucket, object.ObjectName, "", bypassGovernance); err != nil {
+					glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s: %v", bucket, object.ObjectName, err)
+					deleteErrors = append(deleteErrors, DeleteError{
+						Code:    "AccessDenied",
+						Message: err.Error(),
+						Key:     object.ObjectName,
+					})
+					continue
+				}
 			}
 			lastSeparator := strings.LastIndex(object.ObjectName, "/")
 			parentDirectoryPath, entryName, isDeleteData, isRecursive := "", object.ObjectName, true, false

weed/s3api/s3api_object_handlers_put.go

@@ -85,16 +85,15 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request)
 
 		glog.V(1).Infof("PutObjectHandler: bucket %s, object %s, versioningEnabled=%v", bucket, object, versioningEnabled)
 
+		// Check object lock permissions before PUT operation (only for versioned buckets)
+		bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
+		if err := s3a.checkObjectLockPermissionsForPut(bucket, object, bypassGovernance, versioningEnabled); err != nil {
+			s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
+			return
+		}
+
 		if versioningEnabled {
 			// Handle versioned PUT
-			// Check object lock permissions before creating new version (objects under retention should not be overwritten even in versioned buckets)
-			bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
-			if err := s3a.checkObjectLockPermissions(bucket, object, "", bypassGovernance); err != nil {
-				glog.V(2).Infof("PutObjectHandler: object lock check failed for versioned PUT %s/%s: %v", bucket, object, err)
-				s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
-				return
-			}
-
 			glog.V(1).Infof("PutObjectHandler: using versioned PUT for %s/%s", bucket, object)
 			versionId, etag, errCode := s3a.putVersionedObject(r, bucket, object, dataReader, objectContentType)
 			if errCode != s3err.ErrNone {
@@ -111,14 +110,6 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request)
 			setEtag(w, etag)
 		} else {
 			// Handle regular PUT (non-versioned)
-			// Check object lock permissions before overwriting (only for non-versioned buckets)
-			bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
-			if err := s3a.checkObjectLockPermissions(bucket, object, "", bypassGovernance); err != nil {
-				glog.V(2).Infof("PutObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
-				s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
-				return
-			}
-
 			glog.V(1).Infof("PutObjectHandler: using regular PUT for %s/%s", bucket, object)
 			uploadUrl := s3a.toFilerUrl(bucket, object)
 			if objectContentType == "" {

weed/s3api/s3api_object_handlers_retention.go

@@ -17,6 +17,17 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http
 	bucket, object := s3_constants.GetBucketAndObject(r)
 	glog.V(3).Infof("PutObjectRetentionHandler %s %s", bucket, object)
 
+	// Check if Object Lock is available for this bucket (requires versioning)
+	if err := s3a.isObjectLockAvailable(bucket); err != nil {
+		glog.Errorf("PutObjectRetentionHandler: object lock not available for bucket %s: %v", bucket, err)
+		if strings.Contains(err.Error(), "bucket not found") {
+			s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
+		} else {
+			s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
+		}
+		return
+	}
+
 	// Get version ID from query parameters
 	versionId := r.URL.Query().Get("versionId")
 
@@ -71,6 +82,17 @@ func (s3a *S3ApiServer) GetObjectRetentionHandler(w http.ResponseWriter, r *http
 	bucket, object := s3_constants.GetBucketAndObject(r)
 	glog.V(3).Infof("GetObjectRetentionHandler %s %s", bucket, object)
 
+	// Check if Object Lock is available for this bucket (requires versioning)
+	if err := s3a.isObjectLockAvailable(bucket); err != nil {
+		glog.Errorf("GetObjectRetentionHandler: object lock not available for bucket %s: %v", bucket, err)
+		if strings.Contains(err.Error(), "bucket not found") {
+			s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
+		} else {
+			s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
+		}
+		return
+	}
+
 	// Get version ID from query parameters
 	versionId := r.URL.Query().Get("versionId")
 
@@ -129,6 +151,17 @@ func (s3a *S3ApiServer) PutObjectLegalHoldHandler(w http.ResponseWriter, r *http
 	bucket, object := s3_constants.GetBucketAndObject(r)
 	glog.V(3).Infof("PutObjectLegalHoldHandler %s %s", bucket, object)
 
+	// Check if Object Lock is available for this bucket (requires versioning)
+	if err := s3a.isObjectLockAvailable(bucket); err != nil {
+		glog.Errorf("PutObjectLegalHoldHandler: object lock not available for bucket %s: %v", bucket, err)
+		if strings.Contains(err.Error(), "bucket not found") {
+			s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
+		} else {
+			s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
+		}
+		return
+	}
+
 	// Get version ID from query parameters
 	versionId := r.URL.Query().Get("versionId")
 
@@ -175,6 +208,17 @@ func (s3a *S3ApiServer) GetObjectLegalHoldHandler(w http.ResponseWriter, r *http
 	bucket, object := s3_constants.GetBucketAndObject(r)
 	glog.V(3).Infof("GetObjectLegalHoldHandler %s %s", bucket, object)
 
+	// Check if Object Lock is available for this bucket (requires versioning)
+	if err := s3a.isObjectLockAvailable(bucket); err != nil {
+		glog.Errorf("GetObjectLegalHoldHandler: object lock not available for bucket %s: %v", bucket, err)
+		if strings.Contains(err.Error(), "bucket not found") {
+			s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
+		} else {
+			s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
+		}
+		return
+	}
+
 	// Get version ID from query parameters
 	versionId := r.URL.Query().Get("versionId")
 
@@ -242,8 +286,8 @@ func (s3a *S3ApiServer) PutObjectLockConfigurationHandler(w http.ResponseWriter,
 	}
 
 	// Validate object lock configuration
-	if config.ObjectLockEnabled != "" && config.ObjectLockEnabled != s3_constants.ObjectLockEnabled {
-		glog.Errorf("PutObjectLockConfigurationHandler: invalid object lock enabled value: %s", config.ObjectLockEnabled)
+	if err := validateObjectLockConfiguration(config); err != nil {
+		glog.Errorf("PutObjectLockConfigurationHandler: invalid object lock config: %v", err)
 		s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
 		return
 	}

weed/s3api/s3api_object_retention.go

@@ -149,6 +149,56 @@ func validateLegalHold(legalHold *ObjectLegalHold) error {
 	return nil
 }
 
+// validateObjectLockConfiguration validates object lock configuration
+func validateObjectLockConfiguration(config *ObjectLockConfiguration) error {
+	// Validate ObjectLockEnabled if present
+	if config.ObjectLockEnabled != "" && config.ObjectLockEnabled != s3_constants.ObjectLockEnabled {
+		return fmt.Errorf("invalid object lock enabled value: %s", config.ObjectLockEnabled)
+	}
+
+	// Validate Rule if present
+	if config.Rule != nil {
+		if config.Rule.DefaultRetention != nil {
+			return validateDefaultRetention(config.Rule.DefaultRetention)
+		}
+	}
+
+	return nil
+}
+
+// validateDefaultRetention validates default retention configuration
+func validateDefaultRetention(retention *DefaultRetention) error {
+	// Mode is required
+	if retention.Mode == "" {
+		return fmt.Errorf("default retention must specify Mode")
+	}
+
+	// Mode must be valid
+	if retention.Mode != s3_constants.RetentionModeGovernance && retention.Mode != s3_constants.RetentionModeCompliance {
+		return fmt.Errorf("invalid default retention mode: %s", retention.Mode)
+	}
+
+	// Exactly one of Days or Years must be specified
+	if retention.Days == 0 && retention.Years == 0 {
+		return fmt.Errorf("default retention must specify either Days or Years")
+	}
+
+	if retention.Days > 0 && retention.Years > 0 {
+		return fmt.Errorf("default retention cannot specify both Days and Years")
+	}
+
+	// Validate ranges
+	if retention.Days < 0 || retention.Days > 36500 {
+		return fmt.Errorf("default retention days must be between 0 and 36500")
+	}
+
+	if retention.Years < 0 || retention.Years > 100 {
+		return fmt.Errorf("default retention years must be between 0 and 100")
+	}
+
+	return nil
+}
+
 // getObjectRetention retrieves retention configuration from object metadata
 func (s3a *S3ApiServer) getObjectRetention(bucket, object, versionId string) (*ObjectRetention, error) {
 	var entry *filer_pb.Entry
@@ -448,3 +498,37 @@ func (s3a *S3ApiServer) checkObjectLockPermissions(bucket, object, versionId str
 
 	return nil
 }
+
+// isObjectLockAvailable checks if Object Lock features are available for the bucket
+// Object Lock requires versioning to be enabled (AWS S3 requirement)
+func (s3a *S3ApiServer) isObjectLockAvailable(bucket string) error {
+	versioningEnabled, err := s3a.isVersioningEnabled(bucket)
+	if err != nil {
+		if err == filer_pb.ErrNotFound {
+			return fmt.Errorf("bucket not found")
+		}
+		return fmt.Errorf("error checking versioning status: %v", err)
+	}
+
+	if !versioningEnabled {
+		return fmt.Errorf("object lock requires versioning to be enabled")
+	}
+
+	return nil
+}
+
+// checkObjectLockPermissionsForPut checks object lock permissions for PUT operations
+// This is a shared helper to avoid code duplication in PUT handlers
+func (s3a *S3ApiServer) checkObjectLockPermissionsForPut(bucket, object string, bypassGovernance bool, versioningEnabled bool) error {
+	// Object Lock only applies to versioned buckets (AWS S3 requirement)
+	if !versioningEnabled {
+		return nil
+	}
+
+	// For PUT operations, we check permissions on the current object (empty versionId)
+	if err := s3a.checkObjectLockPermissions(bucket, object, "", bypassGovernance); err != nil {
+		glog.V(2).Infof("checkObjectLockPermissionsForPut: object lock check failed for %s/%s: %v", bucket, object, err)
+		return err
+	}
+	return nil
+}

weed/s3api/s3api_object_retention_test.go

@@ -326,7 +326,7 @@ func TestParseXMLGeneric(t *testing.T) {
 			xmlBody: `<?xml version="1.0" encoding="UTF-8"?>
 <Retention xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
     <Mode>GOVERNANCE</Mode>
-    <RetainUntilDate>2024-12-31T23:59:59Z</RetainUntilDate>
+    <RetainUntilDate>2025-01-01T00:00:00Z</RetainUntilDate>
 </Retention>`,
 			expectError: false,
 		},
@@ -334,11 +334,11 @@ func TestParseXMLGeneric(t *testing.T) {
 			name:        "Empty body",
 			xmlBody:     "",
 			expectError: true,
-			errorMsg:    "empty request body",
+			errorMsg:    "error parsing XML",
 		},
 		{
 			name:        "Invalid XML",
-			xmlBody:     "<InvalidXML>",
+			xmlBody:     "not xml",
 			expectError: true,
 			errorMsg:    "error parsing XML",
 		},
@@ -346,13 +346,8 @@ func TestParseXMLGeneric(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			var req *http.Request
-			if tt.xmlBody == "" {
-				req = &http.Request{Body: nil}
-			} else {
-				req = &http.Request{
-					Body: io.NopCloser(strings.NewReader(tt.xmlBody)),
-				}
+			req := &http.Request{
+				Body: io.NopCloser(strings.NewReader(tt.xmlBody)),
 			}
 
 			var retention ObjectRetention
@@ -362,11 +357,242 @@ func TestParseXMLGeneric(t *testing.T) {
 				if err == nil {
 					t.Errorf("Expected error but got none")
 				} else if !strings.Contains(err.Error(), tt.errorMsg) {
-					t.Errorf("Expected error message to contain '%s', got '%s'", tt.errorMsg, err.Error())
+					t.Errorf("Expected error message to contain '%s', got: %v", tt.errorMsg, err)
 				}
 			} else {
 				if err != nil {
-					t.Errorf("Expected no error but got: %v", err)
+					t.Errorf("Unexpected error: %v", err)
+				}
+			}
+		})
+	}
+}
+
+func TestValidateObjectLockConfiguration(t *testing.T) {
+	tests := []struct {
+		name        string
+		config      *ObjectLockConfiguration
+		expectError bool
+		errorMsg    string
+	}{
+		{
+			name: "Valid config with ObjectLockEnabled only",
+			config: &ObjectLockConfiguration{
+				ObjectLockEnabled: "Enabled",
+			},
+			expectError: false,
+		},
+		{
+			name: "Valid config with rule and days",
+			config: &ObjectLockConfiguration{
+				ObjectLockEnabled: "Enabled",
+				Rule: &ObjectLockRule{
+					DefaultRetention: &DefaultRetention{
+						Mode: "GOVERNANCE",
+						Days: 30,
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "Valid config with rule and years",
+			config: &ObjectLockConfiguration{
+				ObjectLockEnabled: "Enabled",
+				Rule: &ObjectLockRule{
+					DefaultRetention: &DefaultRetention{
+						Mode:  "COMPLIANCE",
+						Years: 1,
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "Invalid ObjectLockEnabled value",
+			config: &ObjectLockConfiguration{
+				ObjectLockEnabled: "InvalidValue",
+			},
+			expectError: true,
+			errorMsg:    "invalid object lock enabled value",
+		},
+		{
+			name: "Invalid rule - missing mode",
+			config: &ObjectLockConfiguration{
+				ObjectLockEnabled: "Enabled",
+				Rule: &ObjectLockRule{
+					DefaultRetention: &DefaultRetention{
+						Days: 30,
+					},
+				},
+			},
+			expectError: true,
+			errorMsg:    "default retention must specify Mode",
+		},
+		{
+			name: "Invalid rule - both days and years",
+			config: &ObjectLockConfiguration{
+				ObjectLockEnabled: "Enabled",
+				Rule: &ObjectLockRule{
+					DefaultRetention: &DefaultRetention{
+						Mode:  "GOVERNANCE",
+						Days:  30,
+						Years: 1,
+					},
+				},
+			},
+			expectError: true,
+			errorMsg:    "default retention cannot specify both Days and Years",
+		},
+		{
+			name: "Invalid rule - neither days nor years",
+			config: &ObjectLockConfiguration{
+				ObjectLockEnabled: "Enabled",
+				Rule: &ObjectLockRule{
+					DefaultRetention: &DefaultRetention{
+						Mode: "GOVERNANCE",
+					},
+				},
+			},
+			expectError: true,
+			errorMsg:    "default retention must specify either Days or Years",
+		},
+		{
+			name: "Invalid rule - invalid mode",
+			config: &ObjectLockConfiguration{
+				ObjectLockEnabled: "Enabled",
+				Rule: &ObjectLockRule{
+					DefaultRetention: &DefaultRetention{
+						Mode: "INVALID_MODE",
+						Days: 30,
+					},
+				},
+			},
+			expectError: true,
+			errorMsg:    "invalid default retention mode",
+		},
+		{
+			name: "Invalid rule - days out of range",
+			config: &ObjectLockConfiguration{
+				ObjectLockEnabled: "Enabled",
+				Rule: &ObjectLockRule{
+					DefaultRetention: &DefaultRetention{
+						Mode: "GOVERNANCE",
+						Days: 50000,
+					},
+				},
+			},
+			expectError: true,
+			errorMsg:    "default retention days must be between 0 and 36500",
+		},
+		{
+			name: "Invalid rule - years out of range",
+			config: &ObjectLockConfiguration{
+				ObjectLockEnabled: "Enabled",
+				Rule: &ObjectLockRule{
+					DefaultRetention: &DefaultRetention{
+						Mode:  "GOVERNANCE",
+						Years: 200,
+					},
+				},
+			},
+			expectError: true,
+			errorMsg:    "default retention years must be between 0 and 100",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateObjectLockConfiguration(tt.config)
+
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("Expected error but got none")
+				} else if !strings.Contains(err.Error(), tt.errorMsg) {
+					t.Errorf("Expected error message to contain '%s', got: %v", tt.errorMsg, err)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("Unexpected error: %v", err)
+				}
+			}
+		})
+	}
+}
+
+func TestValidateDefaultRetention(t *testing.T) {
+	tests := []struct {
+		name        string
+		retention   *DefaultRetention
+		expectError bool
+		errorMsg    string
+	}{
+		{
+			name: "Valid retention with days",
+			retention: &DefaultRetention{
+				Mode: "GOVERNANCE",
+				Days: 30,
+			},
+			expectError: false,
+		},
+		{
+			name: "Valid retention with years",
+			retention: &DefaultRetention{
+				Mode:  "COMPLIANCE",
+				Years: 1,
+			},
+			expectError: false,
+		},
+		{
+			name: "Missing mode",
+			retention: &DefaultRetention{
+				Days: 30,
+			},
+			expectError: true,
+			errorMsg:    "default retention must specify Mode",
+		},
+		{
+			name: "Invalid mode",
+			retention: &DefaultRetention{
+				Mode: "INVALID",
+				Days: 30,
+			},
+			expectError: true,
+			errorMsg:    "invalid default retention mode",
+		},
+		{
+			name: "Both days and years specified",
+			retention: &DefaultRetention{
+				Mode:  "GOVERNANCE",
+				Days:  30,
+				Years: 1,
+			},
+			expectError: true,
+			errorMsg:    "default retention cannot specify both Days and Years",
+		},
+		{
+			name: "Neither days nor years specified",
+			retention: &DefaultRetention{
+				Mode: "GOVERNANCE",
+			},
+			expectError: true,
+			errorMsg:    "default retention must specify either Days or Years",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateDefaultRetention(tt.retention)
+
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("Expected error but got none")
+				} else if !strings.Contains(err.Error(), tt.errorMsg) {
+					t.Errorf("Expected error message to contain '%s', got: %v", tt.errorMsg, err)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("Unexpected error: %v", err)
 				}
 			}
 		})