From 0c75395abec2fcc375375e7df7117cb24ccfede5 Mon Sep 17 00:00:00 2001
From: kannappanr <30541348+kannappanr@users.noreply.github.com>
Date: Mon, 22 Apr 2019 07:54:43 -0700
Subject: [PATCH] Fix: Allow deleting multiple objects anonymously if policy
 supports it (#7439)

Fixes #5683
---
 cmd/bucket-handlers.go | 39 ++++++++++++---------------------------
 cmd/web-handlers.go    | 41 ++++++++++++++++++++++++++++++-----------
 2 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go
index 33616151a..695bf3b36 100644
--- a/cmd/bucket-handlers.go
+++ b/cmd/bucket-handlers.go
@@ -263,16 +263,6 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
 		return
 	}
 
-	var s3Error APIErrorCode
-	if s3Error = checkRequestAuthType(ctx, r, policy.DeleteObjectAction, bucket, ""); s3Error != ErrNone {
-		// In the event access is denied, a 200 response should still be returned
-		// http://docs.aws.amazon.com/AmazonS3/latest/API/multiobjectdeleteapi.html
-		if s3Error != ErrAccessDenied {
-			writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r))
-			return
-		}
-	}
-
 	// Content-Length is required and should be non-zero
 	// http://docs.aws.amazon.com/AmazonS3/latest/API/multiobjectdeleteapi.html
 	if r.ContentLength <= 0 {
@@ -324,37 +314,32 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
 		deleteObject = api.CacheAPI().DeleteObject
 	}
 
-	var dErrs = make([]error, len(deleteObjects.Objects))
+	var dErrs = make([]APIErrorCode, len(deleteObjects.Objects))
 	for index, object := range deleteObjects.Objects {
-		// If the request is denied access, each item
-		// should be marked as 'AccessDenied'
-		if s3Error == ErrAccessDenied {
-			dErrs[index] = PrefixAccessDenied{
-				Bucket: bucket,
-				Object: object.ObjectName,
+		if dErrs[index] = checkRequestAuthType(ctx, r, policy.DeleteObjectAction, bucket, object.ObjectName); dErrs[index] != ErrNone {
+			if dErrs[index] == ErrSignatureDoesNotMatch || dErrs[index] == ErrInvalidAccessKeyID {
+				writeErrorResponse(ctx, w, errorCodes.ToAPIErr(dErrs[index]), r.URL, guessIsBrowserReq(r))
+				return
 			}
 			continue
 		}
-		dErrs[index] = deleteObject(ctx, bucket, object.ObjectName)
+		err := deleteObject(ctx, bucket, object.ObjectName)
+		if err != nil {
+			dErrs[index] = toAPIErrorCode(ctx, err)
+		}
 	}
 
 	// Collect deleted objects and errors if any.
 	var deletedObjects []ObjectIdentifier
 	var deleteErrors []DeleteError
-	for index, err := range dErrs {
+	for index, errCode := range dErrs {
 		object := deleteObjects.Objects[index]
 		// Success deleted objects are collected separately.
-		if err == nil {
+		if errCode == ErrNone || errCode == ErrNoSuchKey {
 			deletedObjects = append(deletedObjects, object)
 			continue
 		}
-		if _, ok := err.(ObjectNotFound); ok {
-			// If the object is not found it should be
-			// accounted as deleted as per S3 spec.
-			deletedObjects = append(deletedObjects, object)
-			continue
-		}
-		apiErr := toAPIError(ctx, err)
+		apiErr := getAPIError(errCode)
 		// Error during delete should be collected separately.
 		deleteErrors = append(deleteErrors, DeleteError{
 			Code:    apiErr.Code,
diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go
index b1534f0d2..d69459c0d 100644
--- a/cmd/web-handlers.go
+++ b/cmd/web-handlers.go
@@ -583,7 +583,22 @@ func (web *webAPIHandlers) RemoveObject(r *http.Request, args *RemoveObjectArgs,
 
 	claims, owner, authErr := webRequestAuthenticate(r)
 	if authErr != nil {
-		return toJSONError(authErr)
+		if authErr == errNoAuthToken {
+			// Check if all objects are allowed to be deleted anonymously
+			for _, object := range args.Objects {
+				if !globalPolicySys.IsAllowed(policy.Args{
+					Action:          policy.DeleteObjectAction,
+					BucketName:      args.BucketName,
+					ConditionValues: getConditionValues(r, "", ""),
+					IsOwner:         false,
+					ObjectName:      object,
+				}) {
+					return toJSONError(errAuthentication)
+				}
+			}
+		} else {
+			return toJSONError(authErr)
+		}
 	}
 
 	if args.BucketName == "" || len(args.Objects) == 0 {
@@ -640,16 +655,20 @@ next:
 					return toJSONError(errMethodNotAllowed)
 				}
 			}
-
-			if !globalIAMSys.IsAllowed(iampolicy.Args{
-				AccountName:     claims.Subject,
-				Action:          iampolicy.DeleteObjectAction,
-				BucketName:      args.BucketName,
-				ConditionValues: getConditionValues(r, "", claims.Subject),
-				IsOwner:         owner,
-				ObjectName:      objectName,
-			}) {
-				return toJSONError(errAccessDenied)
+			// Check for permissions only in the case of
+			// non-anonymous login. For anonymous login, policy has already
+			// been checked.
+			if authErr != errNoAuthToken {
+				if !globalIAMSys.IsAllowed(iampolicy.Args{
+					AccountName:     claims.Subject,
+					Action:          iampolicy.DeleteObjectAction,
+					BucketName:      args.BucketName,
+					ConditionValues: getConditionValues(r, "", claims.Subject),
+					IsOwner:         owner,
+					ObjectName:      objectName,
+				}) {
+					return toJSONError(errAccessDenied)
+				}
 			}
 
 			if err = deleteObject(context.Background(), objectAPI, web.CacheAPI(), args.BucketName, objectName, r); err != nil {