feat: adds missing versioning-related bucket policy actions

Closes #1635 Some S3 actions have dedicated bucket policy actions and require explicit policy permissions when operating on object versions. These actions were missing in the gateway: `GetObjectVersionTagging`, `PutObjectVersionTagging`, `DeleteObjectVersionTagging`, `DeleteObjectVersion`, and `GetObjectVersionAttributes`. The logic for these actions is straightforward — if the incoming request includes the `versionId` query parameter, S3 enforces the corresponding bucket policy action that includes `version`. This PR adds support for these missing actions in the gateway.
2026-01-07 12:15:18 +00:00 · 2025-11-12 23:53:27 +04:00
parent 8466d06371
commit 05f8225577
7 changed files with 358 additions and 25 deletions
--- a/auth/bucket_policy_actions.go
+++ b/auth/bucket_policy_actions.go
@@ -38,15 +38,20 @@ const (
 	GetObjectAction                          Action = "s3:GetObject"
 	GetObjectVersionAction                   Action = "s3:GetObjectVersion"
 	DeleteObjectAction                       Action = "s3:DeleteObject"
+	DeleteObjectVersionAction                Action = "s3:DeleteObjectVersion"
 	GetObjectAclAction                       Action = "s3:GetObjectAcl"
 	GetObjectAttributesAction                Action = "s3:GetObjectAttributes"
+	GetObjectVersionAttributesAction         Action = "s3:GetObjectVersionAttributes"
 	PutObjectAclAction                       Action = "s3:PutObjectAcl"
 	RestoreObjectAction                      Action = "s3:RestoreObject"
 	GetBucketTaggingAction                   Action = "s3:GetBucketTagging"
 	PutBucketTaggingAction                   Action = "s3:PutBucketTagging"
 	GetObjectTaggingAction                   Action = "s3:GetObjectTagging"
+	GetObjectVersionTaggingAction            Action = "s3:GetObjectVersionTagging"
 	PutObjectTaggingAction                   Action = "s3:PutObjectTagging"
+	PutObjectVersionTaggingAction            Action = "s3:PutObjectVersionTagging"
 	DeleteObjectTaggingAction                Action = "s3:DeleteObjectTagging"
+	DeleteObjectVersionTaggingAction         Action = "s3:DeleteObjectVersionTagging"
 	ListBucketVersionsAction                 Action = "s3:ListBucketVersions"
 	ListBucketAction                         Action = "s3:ListBucket"
 	GetBucketObjectLockConfigurationAction   Action = "s3:GetBucketObjectLockConfiguration"
@@ -109,15 +114,20 @@ var supportedActionList = map[Action]struct{}{
 	GetObjectAction:                          {},
 	GetObjectVersionAction:                   {},
 	DeleteObjectAction:                       {},
+	DeleteObjectVersionAction:                {},
 	GetObjectAclAction:                       {},
 	GetObjectAttributesAction:                {},
+	GetObjectVersionAttributesAction:         {},
 	PutObjectAclAction:                       {},
 	RestoreObjectAction:                      {},
 	GetBucketTaggingAction:                   {},
 	PutBucketTaggingAction:                   {},
 	GetObjectTaggingAction:                   {},
+	GetObjectVersionTaggingAction:            {},
 	PutObjectTaggingAction:                   {},
+	PutObjectVersionTaggingAction:            {},
 	DeleteObjectTaggingAction:                {},
+	DeleteObjectVersionTaggingAction:         {},
 	ListBucketVersionsAction:                 {},
 	ListBucketAction:                         {},
 	GetBucketObjectLockConfigurationAction:   {},
@@ -163,25 +173,30 @@ var supportedActionList = map[Action]struct{}{
 }

 var supportedObjectActionList = map[Action]struct{}{
-	AbortMultipartUploadAction:      {},
-	ListMultipartUploadPartsAction:  {},
-	PutObjectAction:                 {},
-	GetObjectAction:                 {},
-	GetObjectVersionAction:          {},
-	DeleteObjectAction:              {},
-	GetObjectAclAction:              {},
-	GetObjectAttributesAction:       {},
-	PutObjectAclAction:              {},
-	RestoreObjectAction:             {},
-	GetObjectTaggingAction:          {},
-	PutObjectTaggingAction:          {},
-	DeleteObjectTaggingAction:       {},
-	GetObjectLegalHoldAction:        {},
-	PutObjectLegalHoldAction:        {},
-	GetObjectRetentionAction:        {},
-	PutObjectRetentionAction:        {},
-	BypassGovernanceRetentionAction: {},
-	AllActions:                      {},
+	AbortMultipartUploadAction:       {},
+	ListMultipartUploadPartsAction:   {},
+	PutObjectAction:                  {},
+	GetObjectAction:                  {},
+	GetObjectVersionAction:           {},
+	DeleteObjectAction:               {},
+	DeleteObjectVersionAction:        {},
+	GetObjectAclAction:               {},
+	GetObjectAttributesAction:        {},
+	GetObjectVersionAttributesAction: {},
+	PutObjectAclAction:               {},
+	RestoreObjectAction:              {},
+	GetObjectTaggingAction:           {},
+	GetObjectVersionTaggingAction:    {},
+	PutObjectTaggingAction:           {},
+	PutObjectVersionTaggingAction:    {},
+	DeleteObjectTaggingAction:        {},
+	DeleteObjectVersionTaggingAction: {},
+	GetObjectLegalHoldAction:         {},
+	PutObjectLegalHoldAction:         {},
+	GetObjectRetentionAction:         {},
+	PutObjectRetentionAction:         {},
+	BypassGovernanceRetentionAction:  {},
+	AllActions:                       {},
 }

 // Validates Action: it should either wildcard match with supported actions list or be in it
--- a/s3api/controllers/object-delete.go
+++ b/s3api/controllers/object-delete.go
@@ -36,6 +36,11 @@ func (c S3ApiController) DeleteObjectTagging(ctx *fiber.Ctx) (*Response, error)
 	isBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)

+	action := auth.DeleteObjectTaggingAction
+	if versionId != "" {
+		action = auth.DeleteObjectVersionTaggingAction
+	}
+
 	err := auth.VerifyAccess(ctx.Context(), c.be,
 		auth.AccessOptions{
 			Readonly:        c.readonly,
@@ -45,7 +50,7 @@ func (c S3ApiController) DeleteObjectTagging(ctx *fiber.Ctx) (*Response, error)
 			Acc:             acct,
 			Bucket:          bucket,
 			Object:          key,
-			Action:          auth.DeleteObjectTaggingAction,
+			Action:          action,
 			IsPublicRequest: isBucketPublic,
 		})
 	if err != nil {
@@ -134,7 +139,10 @@ func (c S3ApiController) DeleteObject(ctx *fiber.Ctx) (*Response, error) {
 	isBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)

-	//TODO: check s3:DeleteObjectVersion policy in case a use tries to delete a version of an object
+	action := auth.DeleteObjectAction
+	if versionId != "" {
+		action = auth.DeleteObjectVersionAction
+	}

 	err := auth.VerifyAccess(ctx.Context(), c.be,
 		auth.AccessOptions{
@@ -145,7 +153,7 @@ func (c S3ApiController) DeleteObject(ctx *fiber.Ctx) (*Response, error) {
 			Acc:             acct,
 			Bucket:          bucket,
 			Object:          key,
-			Action:          auth.DeleteObjectAction,
+			Action:          action,
 			IsPublicRequest: isBucketPublic,
 		})
 	if err != nil {
--- a/s3api/controllers/object-get.go
+++ b/s3api/controllers/object-get.go
@@ -41,6 +41,11 @@ func (c S3ApiController) GetObjectTagging(ctx *fiber.Ctx) (*Response, error) {
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
 	isPublicBucket := utils.ContextKeyPublicBucket.IsSet(ctx)

+	action := auth.GetObjectTaggingAction
+	if versionId != "" {
+		action = auth.GetObjectVersionTaggingAction
+	}
+
 	err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{
 		Readonly:        c.readonly,
 		Acl:             parsedAcl,
@@ -49,7 +54,7 @@ func (c S3ApiController) GetObjectTagging(ctx *fiber.Ctx) (*Response, error) {
 		Acc:             acct,
 		Bucket:          bucket,
 		Object:          key,
-		Action:          auth.GetObjectTaggingAction,
+		Action:          action,
 		IsPublicRequest: isPublicBucket,
 	})
 	if err != nil {
@@ -321,6 +326,11 @@ func (c S3ApiController) GetObjectAttributes(ctx *fiber.Ctx) (*Response, error)
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
 	isPublicBucket := utils.ContextKeyPublicBucket.IsSet(ctx)

+	action := auth.GetObjectAttributesAction
+	if versionId != "" {
+		action = auth.GetObjectVersionAttributesAction
+	}
+
 	err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{
 		Readonly:        c.readonly,
 		Acl:             parsedAcl,
@@ -329,7 +339,7 @@ func (c S3ApiController) GetObjectAttributes(ctx *fiber.Ctx) (*Response, error)
 		Acc:             acct,
 		Bucket:          bucket,
 		Object:          key,
-		Action:          auth.GetObjectAttributesAction,
+		Action:          action,
 		IsPublicRequest: isPublicBucket,
 	})
 	if err != nil {
--- a/s3api/controllers/object-put.go
+++ b/s3api/controllers/object-put.go
@@ -42,6 +42,11 @@ func (c S3ApiController) PutObjectTagging(ctx *fiber.Ctx) (*Response, error) {
 	IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)

+	action := auth.PutObjectTaggingAction
+	if versionId != "" {
+		action = auth.PutObjectVersionTaggingAction
+	}
+
 	err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{
 		Readonly:        c.readonly,
 		Acl:             parsedAcl,
@@ -50,7 +55,7 @@ func (c S3ApiController) PutObjectTagging(ctx *fiber.Ctx) (*Response, error) {
 		Acc:             acct,
 		Bucket:          bucket,
 		Object:          key,
-		Action:          auth.PutObjectTaggingAction,
+		Action:          action,
 		IsPublicRequest: IsBucketPublic,
 	})
 	if err != nil {
--- a/tests/integration/group-tests.go
+++ b/tests/integration/group-tests.go
@@ -1071,6 +1071,9 @@ func TestVersioning(ts *TestState) {
 	// Versioninig_concurrent_upload_object
 	ts.Run(Versioning_AccessControl_GetObjectVersion)
 	ts.Run(Versioning_AccessControl_HeadObjectVersion)
+	ts.Run(Versioning_AccessControl_object_tagging_policy)
+	ts.Run(Versioning_AccessControl_DeleteObject_policy)
+	ts.Run(Versioning_AccessControl_GetObjectAttributes_policy)
 }

 func TestVersioningDisabled(ts *TestState) {
@@ -1710,6 +1713,9 @@ func GetIntTests() IntTests {
 		"Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object":         Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object,
 		"Versioning_AccessControl_GetObjectVersion":                               Versioning_AccessControl_GetObjectVersion,
 		"Versioning_AccessControl_HeadObjectVersion":                              Versioning_AccessControl_HeadObjectVersion,
+		"Versioning_AccessControl_object_tagging_policy":                          Versioning_AccessControl_object_tagging_policy,
+		"Versioning_AccessControl_DeleteObject_policy":                            Versioning_AccessControl_DeleteObject_policy,
+		"Versioning_AccessControl_GetObjectAttributes_policy":                     Versioning_AccessControl_GetObjectAttributes_policy,
 		"Versioning_concurrent_upload_object":                                     Versioning_concurrent_upload_object,
 		"RouterPutPartNumberWithoutUploadId":                                      RouterPutPartNumberWithoutUploadId,
 		"RouterPostRoot":                                                          RouterPostRoot,
--- a/tests/integration/utils.go
+++ b/tests/integration/utils.go
@@ -2000,3 +2000,13 @@ type mpinfo struct {
 	uploadId *string
 	parts    []types.CompletedPart
 }
+
+func putBucketPolicy(client *s3.Client, bucket, policy string) error {
+	ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+	_, err := client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
+		Bucket: &bucket,
+		Policy: &policy,
+	})
+	cancel()
+	return err
+}
--- a/tests/integration/versioning.go
+++ b/tests/integration/versioning.go
@@ -2832,6 +2832,285 @@ func Versioning_AccessControl_HeadObjectVersion(s *S3Conf) error {
 	}, withVersioning(types.BucketVersioningStatusEnabled))
 }

+func Versioning_AccessControl_object_tagging_policy(s *S3Conf) error {
+	testName := "Versioning_AccessControl_PutObjectTagging_policy"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		object := "my-object"
+		res, err := putObjectWithData(10, &s3.PutObjectInput{
+			Bucket: &bucket,
+			Key:    &object,
+		}, s3client)
+		if err != nil {
+			return err
+		}
+
+		testuser := getUser("user")
+		err = createUsers(s, []user{testuser})
+		if err != nil {
+			return err
+		}
+
+		userClient := s.getUserClient(testuser)
+
+		putGetDeleteObjectTagging := func(versionId *string, denyAccess bool) error {
+			ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+			_, err := userClient.PutObjectTagging(ctx, &s3.PutObjectTaggingInput{
+				Bucket:    &bucket,
+				Key:       &object,
+				VersionId: versionId,
+				Tagging: &types.Tagging{
+					TagSet: []types.Tag{
+						{Key: getPtr("key"), Value: getPtr("value")},
+					},
+				},
+			})
+			cancel()
+			if denyAccess {
+				if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
+					return err
+				}
+			} else {
+				if err != nil {
+					return err
+				}
+			}
+
+			ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+			_, err = userClient.GetObjectTagging(ctx, &s3.GetObjectTaggingInput{
+				Bucket:    &bucket,
+				Key:       &object,
+				VersionId: versionId,
+			})
+			cancel()
+			if denyAccess {
+				if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
+					return err
+				}
+			} else {
+				if err != nil {
+					return err
+				}
+			}
+
+			ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+			_, err = userClient.DeleteObjectTagging(ctx, &s3.DeleteObjectTaggingInput{
+				Bucket:    &bucket,
+				Key:       &object,
+				VersionId: versionId,
+			})
+			cancel()
+			if denyAccess {
+				if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
+					return err
+				}
+			} else {
+				if err != nil {
+					return err
+				}
+			}
+
+			return nil
+		}
+
+		policy := genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, testuser.access), `["s3:PutObjectVersionTagging", "s3:GetObjectVersionTagging", "s3:DeleteObjectVersionTagging"]`, fmt.Sprintf(`"arn:aws:s3:::%s/*"`, bucket))
+		err = putBucketPolicy(s3client, bucket, policy)
+		if err != nil {
+			return err
+		}
+
+		// deny without versionId
+		err = putGetDeleteObjectTagging(nil, true)
+		if err != nil {
+			return err
+		}
+
+		// allow with versionId
+		err = putGetDeleteObjectTagging(res.res.VersionId, false)
+		if err != nil {
+			return err
+		}
+
+		policy = genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, testuser.access), `["s3:PutObjectTagging", "s3:GetObjectTagging", "s3:DeleteObjectTagging"]`, fmt.Sprintf(`"arn:aws:s3:::%s/*"`, bucket))
+		err = putBucketPolicy(s3client, bucket, policy)
+		if err != nil {
+			return err
+		}
+
+		// allow without versionId
+		err = putGetDeleteObjectTagging(nil, false)
+		if err != nil {
+			return err
+		}
+
+		// deny with versionId
+		err = putGetDeleteObjectTagging(res.res.VersionId, true)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	}, withVersioning(types.BucketVersioningStatusEnabled))
+}
+
+func Versioning_AccessControl_DeleteObject_policy(s *S3Conf) error {
+	testName := "Versioning_AccessControl_DeleteObject_policy"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		obj := "my-object"
+		testuser := getUser("user")
+		err := createUsers(s, []user{testuser})
+		if err != nil {
+			return err
+		}
+
+		userClient := s.getUserClient(testuser)
+
+		delObject := func(versionId *string, denyAccess bool) error {
+			ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+			_, err := userClient.DeleteObject(ctx, &s3.DeleteObjectInput{
+				Bucket:    &bucket,
+				Key:       &obj,
+				VersionId: versionId,
+			})
+			cancel()
+			if denyAccess {
+				return checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied))
+			}
+
+			return err
+		}
+
+		res, err := putObjectWithData(10, &s3.PutObjectInput{
+			Bucket: &bucket,
+			Key:    &obj,
+		}, s3client)
+		if err != nil {
+			return err
+		}
+
+		policy := genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, testuser.access), `"s3:DeleteObject"`, fmt.Sprintf(`"arn:aws:s3:::%s/*"`, bucket))
+		err = putBucketPolicy(s3client, bucket, policy)
+		if err != nil {
+			return err
+		}
+
+		// deny with versionId
+		err = delObject(res.res.VersionId, true)
+		if err != nil {
+			return err
+		}
+
+		// allow without versionId
+		err = delObject(nil, false)
+		if err != nil {
+			return err
+		}
+
+		policy = genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, testuser.access), `"s3:DeleteObjectVersion"`, fmt.Sprintf(`"arn:aws:s3:::%s/*"`, bucket))
+		err = putBucketPolicy(s3client, bucket, policy)
+		if err != nil {
+			return err
+		}
+
+		// recreate the object
+		res, err = putObjectWithData(10, &s3.PutObjectInput{
+			Bucket: &bucket,
+			Key:    &obj,
+		}, s3client)
+		if err != nil {
+			return err
+		}
+
+		// deny without versionId
+		err = delObject(nil, true)
+		if err != nil {
+			return err
+		}
+
+		// allow with versionId
+		err = delObject(res.res.VersionId, false)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	}, withVersioning(types.BucketVersioningStatusEnabled))
+}
+
+func Versioning_AccessControl_GetObjectAttributes_policy(s *S3Conf) error {
+	testName := "Versioning_AccessControl_GetObjectAttributes_policy"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		obj := "my-object"
+		res, err := putObjectWithData(10, &s3.PutObjectInput{
+			Bucket: &bucket,
+			Key:    &obj,
+		}, s3client)
+		if err != nil {
+			return err
+		}
+
+		testuser := getUser("user")
+		err = createUsers(s, []user{testuser})
+		if err != nil {
+			return err
+		}
+		userClient := s.getUserClient(testuser)
+
+		getObjectAttr := func(versionId *string, denyAccess bool) error {
+			ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+			_, err := userClient.GetObjectAttributes(ctx, &s3.GetObjectAttributesInput{
+				Bucket:           &bucket,
+				Key:              &obj,
+				VersionId:        versionId,
+				ObjectAttributes: types.ObjectAttributesChecksum.Values(),
+			})
+			cancel()
+			if denyAccess {
+				return checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied))
+			}
+
+			return nil
+		}
+
+		policy := genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, testuser.access), `"s3:GetObjectAttributes"`, fmt.Sprintf(`"arn:aws:s3:::%s/*"`, bucket))
+		err = putBucketPolicy(s3client, bucket, policy)
+		if err != nil {
+			return err
+		}
+
+		// deny with versionId
+		err = getObjectAttr(res.res.VersionId, true)
+		if err != nil {
+			return err
+		}
+
+		// allow without versionId
+		err = getObjectAttr(nil, false)
+		if err != nil {
+			return err
+		}
+
+		policy = genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, testuser.access), `"s3:GetObjectVersionAttributes"`, fmt.Sprintf(`"arn:aws:s3:::%s/*"`, bucket))
+		err = putBucketPolicy(s3client, bucket, policy)
+		if err != nil {
+			return err
+		}
+
+		// deny without versionId
+		err = getObjectAttr(nil, true)
+		if err != nil {
+			return err
+		}
+
+		// allow with versionId
+		err = getObjectAttr(res.res.VersionId, false)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	}, withVersioning(types.BucketVersioningStatusEnabled))
+}
+
 func VersioningDisabled_GetBucketVersioning_not_configured(s *S3Conf) error {
 	testName := "VersioningDisabled_GetBucketVersioning_not_configured"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {