diff --git a/cmd/bucket-policy.go b/cmd/bucket-policy.go index c03e90274..7f62dd9d5 100644 --- a/cmd/bucket-policy.go +++ b/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) diff --git a/cmd/object-common.go b/cmd/object-common.go index fbb9a517f..63009de35 100644 --- a/cmd/object-common.go +++ b/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 +} diff --git a/cmd/web-handlers_test.go b/cmd/web-handlers_test.go index c7d759c79..1a0c2f72c 100644 --- a/cmd/web-handlers_test.go +++ b/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: %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) + } + } +}