From 3d21119ec8c5d9f71763721129422953be5554a7 Mon Sep 17 00:00:00 2001
From: wd256 <wd256@users.noreply.github.com>
Date: Wed, 16 Aug 2017 05:49:31 +1000
Subject: [PATCH] Provide 200 response with per object error listing on access
 denied for delete multiple object request (#4817)

---
 cmd/bucket-handlers.go      | 22 ++++++++++++++++----
 cmd/bucket-handlers_test.go | 40 ++++++++++++++++++++++++++++++++-----
 2 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go
index 4400f4ce6..3ab10a529 100644
--- a/cmd/bucket-handlers.go
+++ b/cmd/bucket-handlers.go
@@ -240,9 +240,14 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
 		return
 	}
 
-	if s3Error := checkRequestAuthType(r, bucket, "s3:DeleteObject", serverConfig.GetRegion()); s3Error != ErrNone {
-		writeErrorResponse(w, s3Error, r.URL)
-		return
+	var authError APIErrorCode
+	if authError = checkRequestAuthType(r, bucket, "s3:DeleteObject", serverConfig.GetRegion()); authError != 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 authError != ErrAccessDenied {
+			writeErrorResponse(w, authError, r.URL)
+			return
+		}
 	}
 
 	// Content-Length is required and should be non-zero
@@ -284,10 +289,19 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
 	for index, object := range deleteObjects.Objects {
 		wg.Add(1)
 		go func(i int, obj ObjectIdentifier) {
+			defer wg.Done()
+			// If the request is denied access, each item
+			// should be marked as 'AccessDenied'
+			if authError == ErrAccessDenied {
+				dErrs[i] = PrefixAccessDenied{
+					Bucket: bucket,
+					Object: obj.ObjectName,
+				}
+				return
+			}
 			objectLock := globalNSMutex.NewNSLock(bucket, obj.ObjectName)
 			objectLock.Lock()
 			defer objectLock.Unlock()
-			defer wg.Done()
 
 			dErr := objectAPI.DeleteObject(bucket, obj.ObjectName)
 			if dErr != nil {
diff --git a/cmd/bucket-handlers_test.go b/cmd/bucket-handlers_test.go
index bd57e7eb9..bce2b66e9 100644
--- a/cmd/bucket-handlers_test.go
+++ b/cmd/bucket-handlers_test.go
@@ -650,6 +650,17 @@ func testAPIDeleteMultipleObjectsHandler(obj ObjectLayer, instanceType, bucketNa
 
 		return objectIdentifierList
 	}
+	getDeleteErrorList := func(objects []ObjectIdentifier) (deleteErrorList []DeleteError) {
+		for _, obj := range objects {
+			deleteErrorList = append(deleteErrorList, DeleteError{
+				Code:    errorCodeResponse[ErrAccessDenied].Code,
+				Message: errorCodeResponse[ErrAccessDenied].Description,
+				Key:     obj.ObjectName,
+			})
+		}
+
+		return deleteErrorList
+	}
 
 	requestList := []DeleteObjectsRequest{
 		{Quiet: false, Objects: getObjectIdentifierList(objectNames[:5])},
@@ -670,6 +681,10 @@ func testAPIDeleteMultipleObjectsHandler(obj ObjectLayer, instanceType, bucketNa
 	errorResponse := generateMultiDeleteResponse(requestList[1].Quiet, requestList[1].Objects, nil)
 	encodedErrorResponse := encodeResponse(errorResponse)
 
+	anonRequest := encodeResponse(requestList[0])
+	anonResponse := generateMultiDeleteResponse(requestList[0].Quiet, nil, getDeleteErrorList(requestList[0].Objects))
+	encodedAnonResponse := encodeResponse(anonResponse)
+
 	testCases := []struct {
 		bucket             string
 		objects            []byte
@@ -718,15 +733,32 @@ func testAPIDeleteMultipleObjectsHandler(obj ObjectLayer, instanceType, bucketNa
 			expectedContent:    encodedErrorResponse,
 			expectedRespStatus: http.StatusOK,
 		},
+		// Test case - 5.
+		// Anonymous user access denied response
+		// Currently anonymous users cannot delete multiple objects in Minio server
+		{
+			bucket:             bucketName,
+			objects:            anonRequest,
+			accessKey:          "",
+			secretKey:          "",
+			expectedContent:    encodedAnonResponse,
+			expectedRespStatus: http.StatusOK,
+		},
 	}
 
 	for i, testCase := range testCases {
 		var req *http.Request
 		var actualContent []byte
 
-		// Indicating that all parts are uploaded and initiating completeMultipartUpload.
-		req, err = newTestSignedRequestV4("POST", getDeleteMultipleObjectsURL("", bucketName),
-			int64(len(testCase.objects)), bytes.NewReader(testCase.objects), testCase.accessKey, testCase.secretKey)
+		// Generate a signed or anonymous request based on the testCase
+		if testCase.accessKey != "" {
+			req, err = newTestSignedRequestV4("POST", getDeleteMultipleObjectsURL("", bucketName),
+				int64(len(testCase.objects)), bytes.NewReader(testCase.objects), testCase.accessKey, testCase.secretKey)
+		} else {
+			req, err = newTestRequest("POST", getDeleteMultipleObjectsURL("", bucketName),
+				int64(len(testCase.objects)), bytes.NewReader(testCase.objects))
+		}
+
 		if err != nil {
 			t.Fatalf("Failed to create HTTP request for DeleteMultipleObjects: <ERROR> %v", err)
 		}
@@ -753,8 +785,6 @@ func testAPIDeleteMultipleObjectsHandler(obj ObjectLayer, instanceType, bucketNa
 		}
 	}
 
-	// Currently anonymous user cannot delete multiple objects in Minio server, hence no test case is required.
-
 	// HTTP request to test the case of `objectLayer` being set to `nil`.
 	// There is no need to use an existing bucket or valid input for creating the request,
 	// since the `objectLayer==nil`  check is performed before any other checks inside the handlers.