Check for bucket existence in Set/Get/Remove bucket policy workflow + tests (#2745)

cmd/bucket-policy.go

@@ -134,10 +134,11 @@ func getOldBucketsConfigPath() (string, error) {
 // readBucketPolicyJSON - reads bucket policy for an input bucket, returns BucketPolicyNotFound
 // if bucket policy is not found.
 func readBucketPolicyJSON(bucket string, objAPI ObjectLayer) (bucketPolicyReader io.Reader, err error) {
-	// Verify bucket is valid.
-	if !IsValidBucketName(bucket) {
-		return nil, BucketNameInvalid{Bucket: bucket}
+	// Verify if bucket actually exists
+	if e := isBucketExist(bucket, objAPI); e != nil {
+		return nil, e
 	}
+
 	policyPath := pathJoin(bucketConfigPrefix, bucket, policyJSON)
 	objInfo, err := objAPI.GetObjectInfo(minioMetaBucket, policyPath)
 	err = errorCause(err)
@@ -184,10 +185,11 @@ func readBucketPolicy(bucket string, objAPI ObjectLayer) (*bucketPolicy, error)
 // removeBucketPolicy - removes any previously written bucket policy. Returns BucketPolicyNotFound
 // if no policies are found.
 func removeBucketPolicy(bucket string, objAPI ObjectLayer) error {
-	// Verify bucket is valid.
-	if !IsValidBucketName(bucket) {
-		return BucketNameInvalid{Bucket: bucket}
+	// Verify if bucket actually exists
+	if err := isBucketExist(bucket, objAPI); err != nil {
+		return err
 	}
+
 	policyPath := pathJoin(bucketConfigPrefix, bucket, policyJSON)
 	if err := objAPI.DeleteObject(minioMetaBucket, policyPath); err != nil {
 		errorIf(err, "Unable to remove bucket-policy on bucket %s.", bucket)
@@ -202,9 +204,9 @@ func removeBucketPolicy(bucket string, objAPI ObjectLayer) error {
 
 // writeBucketPolicy - save all bucket policies.
 func writeBucketPolicy(bucket string, objAPI ObjectLayer, reader io.Reader, size int64) error {
-	// Verify if bucket path legal
-	if !IsValidBucketName(bucket) {
-		return BucketNameInvalid{Bucket: bucket}
+	// Verify if bucket actually exists
+	if err := isBucketExist(bucket, objAPI); err != nil {
+		return err
 	}
 
 	policyPath := pathJoin(bucketConfigPrefix, bucket, policyJSON)

cmd/object-common.go

@@ -240,3 +240,15 @@ func cleanupDir(storage StorageAPI, volume, dirPath string) error {
 	err := delFunc(retainSlash(pathJoin(dirPath)))
 	return err
 }
+
+// Checks whether bucket exists.
+func isBucketExist(bucket string, obj ObjectLayer) error {
+	if !IsValidBucketName(bucket) {
+		return BucketNameInvalid{Bucket: bucket}
+	}
+	_, err := obj.GetBucketInfo(bucket)
+	if err != nil {
+		return BucketNotFound{Bucket: bucket}
+	}
+	return nil
+}

cmd/web-handlers_test.go

@@ -742,6 +742,9 @@ func testWebGetBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestE
 	rec := httptest.NewRecorder()
 
 	bucketName := getRandomBucketName()
+	if err = obj.MakeBucket(bucketName); err != nil {
+		t.Fatal("Unexpected error: ", err)
+	}
 
 	testCases := []struct {
 		bucketName     string
@@ -770,3 +773,76 @@ func testWebGetBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestE
 		}
 	}
 }
+
+// Wrapper for calling SetBucketPolicy Handler
+func TestWebHandlerSetBucketPolicyHandler(t *testing.T) {
+	ExecObjectLayerTest(t, testWebSetBucketPolicyHandler)
+}
+
+// testWebSetBucketPolicyHandler - Test SetBucketPolicy web handler
+func testWebSetBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestErrHandler) {
+	// Register the API end points with XL/FS object layer.
+	apiRouter := initTestWebRPCEndPoint(obj)
+	// initialize the server and obtain the credentials and root.
+	// credentials are necessary to sign the HTTP request.
+	rootPath, err := newTestConfig("us-east-1")
+	if err != nil {
+		t.Fatalf("Init Test config failed")
+	}
+	// remove the root folder after the test ends.
+	defer removeAll(rootPath)
+
+	credentials := serverConfig.GetCredential()
+
+	authorization, err := getWebRPCToken(apiRouter, credentials.AccessKeyID, credentials.SecretAccessKey)
+	if err != nil {
+		t.Fatal("Cannot authenticate")
+	}
+
+	rec := httptest.NewRecorder()
+
+	// Create a bucket
+	bucketName := getRandomBucketName()
+	if err = obj.MakeBucket(bucketName); err != nil {
+		t.Fatal("Unexpected error: ", err)
+	}
+
+	testCases := []struct {
+		bucketName string
+		prefix     string
+		policy     string
+		pass       bool
+	}{
+		// Inexistent bucket
+		{"fooo", "", "readonly", false},
+		// Invalid bucket name
+		{"", "", "readonly", false},
+		// Invalid policy
+		{bucketName, "", "foo", false},
+		// Valid parameters
+		{bucketName, "", "readwrite", true},
+	}
+
+	for i, testCase := range testCases {
+		args := &SetBucketPolicyArgs{BucketName: testCase.bucketName, Prefix: testCase.prefix, Policy: testCase.policy}
+		reply := &WebGenericRep{}
+		// Call SetBucketPolicy RPC
+		req, err := newTestWebRPCRequest("Web.SetBucketPolicy", authorization, args)
+		if err != nil {
+			t.Fatalf("Test %d: Failed to create HTTP request: <ERROR> %v", i+1, err)
+		}
+		apiRouter.ServeHTTP(rec, req)
+		// Check if we have 200 OK
+		if rec.Code != http.StatusOK {
+			t.Fatalf("Test %d: Expected the response status to be 200, but instead found `%d`", i+1, rec.Code)
+		}
+		// Parse RPC response
+		err = getTestWebRPCResponse(rec, &reply)
+		if testCase.pass && err != nil {
+			t.Fatalf("Test %d: Should succeed but it didn't, %v", i+1, err)
+		}
+		if !testCase.pass && err == nil {
+			t.Fatalf("Test %d: Should fail it didn't", i+1)
+		}
+	}
+}