fix: multiObjectDelete by passing versionId for authorization (#16562)

cmd/admin-handlers-users_test.go

@@ -1261,6 +1261,20 @@ func (c *check) mustListBuckets(ctx context.Context, client *minio.Client) {
 	}
 }
 
+func (c *check) mustNotDelete(ctx context.Context, client *minio.Client, bucket string, vid string) {
+	c.Helper()
+
+	err := client.RemoveObject(ctx, bucket, "some-object", minio.RemoveObjectOptions{VersionID: vid})
+	if err == nil {
+		c.Fatalf("user must not be allowed to delete")
+	}
+
+	err = client.RemoveObject(ctx, bucket, "some-object", minio.RemoveObjectOptions{})
+	if err != nil {
+		c.Fatal("user must be able to create delete marker")
+	}
+}
+
 func (c *check) mustDownload(ctx context.Context, client *minio.Client, bucket string) {
 	c.Helper()
 	rd, err := client.GetObject(ctx, bucket, "some-object", minio.GetObjectOptions{})
@@ -1272,6 +1286,19 @@ func (c *check) mustDownload(ctx context.Context, client *minio.Client, bucket s
 	}
 }
 
+func (c *check) mustUploadReturnVersions(ctx context.Context, client *minio.Client, bucket string) []string {
+	c.Helper()
+	versions := []string{}
+	for i := 0; i < 5; i++ {
+		ui, err := client.PutObject(ctx, bucket, "some-object", bytes.NewBuffer([]byte("stuff")), 5, minio.PutObjectOptions{})
+		if err != nil {
+			c.Fatalf("upload did not succeed got %#v", err)
+		}
+		versions = append(versions, ui.VersionID)
+	}
+	return versions
+}
+
 func (c *check) mustUpload(ctx context.Context, client *minio.Client, bucket string) {
 	c.Helper()
 	_, err := client.PutObject(ctx, bucket, "some-object", bytes.NewBuffer([]byte("stuff")), 5, minio.PutObjectOptions{})

cmd/auth-handler.go

@@ -300,6 +300,17 @@ func checkRequestAuthType(ctx context.Context, r *http.Request, action policy.Ac
 	return s3Err
 }
 
+// checkRequestAuthTypeWithVID is similar to checkRequestAuthType
+// passes versionID additionally.
+func checkRequestAuthTypeWithVID(ctx context.Context, r *http.Request, action policy.Action, bucketName, objectName, versionID string) (s3Err APIErrorCode) {
+	logger.GetReqInfo(ctx).BucketName = bucketName
+	logger.GetReqInfo(ctx).ObjectName = objectName
+	logger.GetReqInfo(ctx).VersionID = versionID
+
+	_, _, s3Err = checkRequestAuthTypeCredential(ctx, r, action)
+	return s3Err
+}
+
 func authenticateRequest(ctx context.Context, r *http.Request, action policy.Action) (s3Err APIErrorCode) {
 	if logger.GetReqInfo(ctx) == nil {
 		logger.LogIf(ctx, errors.New("unexpected context.Context does not have a logger.ReqInfo"), logger.Minio)

cmd/bucket-handlers.go

@@ -495,7 +495,7 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
 	vc, _ := globalBucketVersioningSys.Get(bucket)
 	oss := make([]*objSweeper, len(deleteObjectsReq.Objects))
 	for index, object := range deleteObjectsReq.Objects {
-		if apiErrCode := checkRequestAuthType(ctx, r, policy.DeleteObjectAction, bucket, object.ObjectName); apiErrCode != ErrNone {
+		if apiErrCode := checkRequestAuthTypeWithVID(ctx, r, policy.DeleteObjectAction, bucket, object.ObjectName, object.VersionID); apiErrCode != ErrNone {
 			if apiErrCode == ErrSignatureDoesNotMatch || apiErrCode == ErrInvalidAccessKeyID {
 				writeErrorResponse(ctx, w, errorCodes.ToAPIErr(apiErrCode), r.URL)
 				return

cmd/sts-handlers_test.go

@@ -36,6 +36,7 @@ func runAllIAMSTSTests(suite *TestSuiteIAM, c *check) {
 	// The STS for root test needs to be the first one after setup.
 	suite.TestSTSForRoot(c)
 	suite.TestSTS(c)
+	suite.TestSTSWithDenyDeleteVersion(c)
 	suite.TestSTSWithTags(c)
 	suite.TestSTSWithGroupPolicy(c)
 	suite.TearDownSuite(c)
@@ -73,6 +74,101 @@ func TestIAMInternalIDPSTSServerSuite(t *testing.T) {
 	}
 }
 
+func (s *TestSuiteIAM) TestSTSWithDenyDeleteVersion(c *check) {
+	ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout)
+	defer cancel()
+
+	bucket := getRandomBucketName()
+	err := s.client.MakeBucket(ctx, bucket, minio.MakeBucketOptions{ObjectLocking: true})
+	if err != nil {
+		c.Fatalf("bucket creat error: %v", err)
+	}
+
+	// Create policy, user and associate policy
+	policy := "mypolicy"
+	policyBytes := []byte(fmt.Sprintf(`{
+  "Version": "2012-10-17",
+  "Statement": [
+   {
+    "Sid": "ObjectActionsRW",
+    "Effect": "Allow",
+    "Action": [
+     "s3:PutObject",
+     "s3:PutObjectTagging",
+     "s3:AbortMultipartUpload",
+     "s3:DeleteObject",
+     "s3:GetObject",
+     "s3:GetObjectTagging",
+     "s3:GetObjectVersion",
+     "s3:ListMultipartUploadParts"
+    ],
+    "Resource": [
+     "arn:aws:s3:::%s/*"
+    ]
+   },
+   {
+    "Sid": "DenyDeleteVersionAction",
+    "Effect": "Deny",
+    "Action": [
+     "s3:DeleteObjectVersion"
+    ],
+    "Resource": [
+     "arn:aws:s3:::%s/*"
+    ]
+   }
+  ]
+ }
+`, bucket, bucket))
+
+	err = s.adm.AddCannedPolicy(ctx, policy, policyBytes)
+	if err != nil {
+		c.Fatalf("policy add error: %v", err)
+	}
+
+	accessKey, secretKey := mustGenerateCredentials(c)
+	err = s.adm.SetUser(ctx, accessKey, secretKey, madmin.AccountEnabled)
+	if err != nil {
+		c.Fatalf("Unable to set user: %v", err)
+	}
+
+	err = s.adm.SetPolicy(ctx, policy, accessKey, false)
+	if err != nil {
+		c.Fatalf("Unable to set policy: %v", err)
+	}
+
+	// confirm that the user is able to access the bucket
+	uClient := s.getUserClient(c, accessKey, secretKey, "")
+	versions := c.mustUploadReturnVersions(ctx, uClient, bucket)
+	c.mustNotDelete(ctx, uClient, bucket, versions[0])
+
+	assumeRole := cr.STSAssumeRole{
+		Client:      s.TestSuiteCommon.client,
+		STSEndpoint: s.endPoint,
+		Options: cr.STSAssumeRoleOptions{
+			AccessKey: accessKey,
+			SecretKey: secretKey,
+			Location:  "",
+		},
+	}
+
+	value, err := assumeRole.Retrieve()
+	if err != nil {
+		c.Fatalf("err calling assumeRole: %v", err)
+	}
+
+	minioClient, err := minio.New(s.endpoint, &minio.Options{
+		Creds:     cr.NewStaticV4(value.AccessKeyID, value.SecretAccessKey, value.SessionToken),
+		Secure:    s.secure,
+		Transport: s.TestSuiteCommon.client.Transport,
+	})
+	if err != nil {
+		c.Fatalf("Error initializing client: %v", err)
+	}
+
+	versions = c.mustUploadReturnVersions(ctx, minioClient, bucket)
+	c.mustNotDelete(ctx, minioClient, bucket, versions[0])
+}
+
 func (s *TestSuiteIAM) TestSTSWithTags(c *check) {
 	ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout)
 	defer cancel()
@@ -171,9 +267,9 @@ func (s *TestSuiteIAM) TestSTSWithTags(c *check) {
 	}
 
 	// Validate sts creds can access the object
-	c.mustPutObjectWithTags(ctx, uClient, bucket, object)
+	c.mustPutObjectWithTags(ctx, minioClient, bucket, object)
-	c.mustGetObject(ctx, uClient, bucket, object)
+	c.mustGetObject(ctx, minioClient, bucket, object)
-	c.mustHeadObject(ctx, uClient, bucket, object, 2)
+	c.mustHeadObject(ctx, minioClient, bucket, object, 2)
 
 	// Validate that the client can remove objects
 	if err = minioClient.RemoveObjectTagging(ctx, bucket, object, minio.RemoveObjectTaggingOptions{}); err != nil {