From e213172431d481b25e1dbb11c90a4dc5c880c99a Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Sun, 9 Oct 2016 21:51:37 +0530 Subject: [PATCH] tests: Missing anonymous tests for bucket-handlers. (#2885) --- cmd/bucket-handlers.go | 3 +- cmd/bucket-handlers_test.go | 164 ++++++++++++++---------------------- cmd/test-utils_test.go | 25 +++--- 3 files changed, 80 insertions(+), 112 deletions(-) diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 5daf259e2..1f29c5ad7 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -199,7 +199,7 @@ func (api objectAPIHandlers) ListMultipartUploadsHandler(w http.ResponseWriter, writeSuccessResponse(w, encodedSuccessResponse) } -// ListBucketsHandler - GET Service +// ListBucketsHandler - GET Service. // ----------- // This implementation of the GET operation returns a list of all buckets // owned by the authenticated sender of the request. @@ -502,7 +502,6 @@ func (api objectAPIHandlers) HeadBucketHandler(w http.ResponseWriter, r *http.Re writeErrorResponse(w, r, ErrServerNotInitialized, r.URL.Path) return } - switch getRequestAuthType(r) { default: // For all unknown auth types return error. diff --git a/cmd/bucket-handlers_test.go b/cmd/bucket-handlers_test.go index f7616cdbb..d72ec035e 100644 --- a/cmd/bucket-handlers_test.go +++ b/cmd/bucket-handlers_test.go @@ -26,32 +26,13 @@ import ( // Wrapper for calling GetBucketPolicy HTTP handler tests for both XL multiple disks and single node setup. func TestGetBucketLocationHandler(t *testing.T) { - ExecObjectLayerTest(t, testGetBucketLocationHandler) + ExecObjectLayerAPITest(t, testGetBucketLocationHandler, []string{"GetBucketLocation"}) } -func testGetBucketLocationHandler(obj ObjectLayer, instanceType string, t TestErrHandler) { +func testGetBucketLocationHandler(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, + credentials credential, t *testing.T) { initBucketPolicies(obj) - // get random bucket name. - bucketName := getRandomBucketName() - // Create bucket. - err := obj.MakeBucket(bucketName) - if err != nil { - // failed to create newbucket, abort. - t.Fatalf("%s : %s", instanceType, err) - } - // Register the API end points with XL/FS object layer. - apiRouter := initTestAPIEndPoints(obj, []string{"GetBucketLocation"}) - // 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() // test cases with sample input and expected output. testCases := []struct { bucketName string @@ -122,36 +103,29 @@ func testGetBucketLocationHandler(obj ObjectLayer, instanceType string, t TestEr t.Errorf("Test %d: %s: Expected the error code to be `%s`, but instead found `%s`", i+1, instanceType, testCase.errorResponse.Code, errorResponse.Code) } } + + // Test for Anonymous/unsigned http request. + // ListBucketsHandler doesn't support bucket policies, setting the policies shouldn't make any difference. + anonReq, err := newTestRequest("GET", getBucketLocationURL("", bucketName), 0, nil) + if err != nil { + t.Fatalf("Minio %s: Failed to create an anonymous request.", instanceType) + } + + // ExecObjectLayerAPIAnonTest - Calls the HTTP API handler using the anonymous request, validates the ErrAccessDeniedResponse, + // sets the bucket policy using the policy statement generated from `getReadOnlyBucketStatement` so that the + // unsigned request goes through and its validated again. + ExecObjectLayerAPIAnonTest(t, "TestGetBucketLocationHandler", bucketName, "", instanceType, apiRouter, anonReq, getReadOnlyBucketStatement) } // Wrapper for calling HeadBucket HTTP handler tests for both XL multiple disks and single node setup. func TestHeadBucketHandler(t *testing.T) { - ExecObjectLayerTest(t, testHeadBucketHandler) + ExecObjectLayerAPITest(t, testHeadBucketHandler, []string{"HeadBucket"}) } -func testHeadBucketHandler(obj ObjectLayer, instanceType string, t TestErrHandler) { +func testHeadBucketHandler(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, + credentials credential, t *testing.T) { initBucketPolicies(obj) - // get random bucket name. - bucketName := getRandomBucketName() - // Create bucket. - err := obj.MakeBucket(bucketName) - if err != nil { - // failed to create newbucket, abort. - t.Fatalf("%s : %s", instanceType, err) - } - // Register the API end points with XL/FS object layer. - apiRouter := initTestAPIEndPoints(obj, []string{"HeadBucket"}) - // 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() // test cases with sample input and expected output. testCases := []struct { bucketName string @@ -198,43 +172,31 @@ func testHeadBucketHandler(obj ObjectLayer, instanceType string, t TestErrHandle t.Errorf("Test %d: %s: Expected the response status to be `%d`, but instead found `%d`", i+1, instanceType, testCase.expectedRespStatus, rec.Code) } } + + // Test for Anonymous/unsigned http request. + anonReq, err := newTestRequest("HEAD", getHEADBucketURL("", bucketName), 0, nil) + + if err != nil { + t.Fatalf("Minio %s: Failed to create an anonymous request for bucket \"%s\": %v", + instanceType, bucketName, err) + } + + // ExecObjectLayerAPIAnonTest - Calls the HTTP API handler using the anonymous request, validates the ErrAccessDeniedResponse, + // sets the bucket policy using the policy statement generated from `getReadOnlyBucketStatement` so that the + // unsigned request goes through and its validated again. + ExecObjectLayerAPIAnonTest(t, "TestHeadBucketHandler", bucketName, "", instanceType, apiRouter, anonReq, getReadOnlyBucketStatement) } // Wrapper for calling TestListMultipartUploadsHandler tests for both XL multiple disks and single node setup. func TestListMultipartUploadsHandler(t *testing.T) { - ExecObjectLayerTest(t, testListMultipartUploadsHandler) + ExecObjectLayerAPITest(t, testListMultipartUploadsHandler, []string{"ListMultipartUploads"}) } // testListMultipartUploadsHandler - Tests validate listing of multipart uploads. -func testListMultipartUploadsHandler(obj ObjectLayer, instanceType string, t TestErrHandler) { +func testListMultipartUploadsHandler(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, + credentials credential, t *testing.T) { initBucketPolicies(obj) - // get random bucket name. - bucketName := getRandomBucketName() - - // Register the API end points with XL/FS object layer. - apiRouter := initTestAPIEndPoints(obj, []string{"ListMultipartUploads"}) - // 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() - - // bucketnames[0]. - // objectNames[0]. - // uploadIds [0]. - // Create bucket before initiating NewMultipartUpload. - err = obj.MakeBucket(bucketName) - if err != nil { - // Failed to create newbucket, abort. - t.Fatalf("%s : %s", instanceType, err.Error()) - } - // Collection of non-exhaustive ListMultipartUploads test cases, valid errors // and success responses. testCases := []struct { @@ -299,40 +261,31 @@ func testListMultipartUploadsHandler(obj ObjectLayer, instanceType string, t Tes if rec.Code != http.StatusForbidden { t.Errorf("Test %s: Expected the response status to be `http.StatusForbidden`, but instead found `%d`", instanceType, rec.Code) } + + url := getListMultipartUploadsURLWithParams("", testCases[6].bucket, testCases[6].prefix, testCases[6].keyMarker, + testCases[6].uploadIDMarker, testCases[6].delimiter, testCases[6].maxUploads) + // Test for Anonymous/unsigned http request. + anonReq, err := newTestRequest("GET", url, 0, nil) + if err != nil { + t.Fatalf("Minio %s: Failed to create an anonymous request for bucket \"%s\": %v", + instanceType, bucketName, err) + } + + // ExecObjectLayerAPIAnonTest - Calls the HTTP API handler using the anonymous request, validates the ErrAccessDeniedResponse, + // sets the bucket policy using the policy statement generated from `getWriteOnlyBucketStatement` so that the + // unsigned request goes through and its validated again. + ExecObjectLayerAPIAnonTest(t, "TestListMultipartUploadsHandler", bucketName, "", instanceType, apiRouter, anonReq, getWriteOnlyBucketStatement) + } // Wrapper for calling TestListBucketsHandler tests for both XL multiple disks and single node setup. func TestListBucketsHandler(t *testing.T) { - ExecObjectLayerTest(t, testListBuckets) + ExecObjectLayerAPITest(t, testListBucketsHandler, []string{"ListBuckets"}) } // testListBucketsHandler - Tests validate listing of buckets. -func testListBucketsHandler(obj ObjectLayer, instanceType string, t TestErrHandler) { - // get random bucket name. - bucketName := getRandomBucketName() - - // Register the API end points with XL/FS object layer. - apiRouter := initTestAPIEndPoints(obj, []string{"ListBuckets"}) - // 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() - - // bucketnames[0]. - // objectNames[0]. - // uploadIds [0]. - // Create bucket before initiating NewMultipartUpload. - err = obj.MakeBucket(bucketName) - if err != nil { - // Failed to create newbucket, abort. - t.Fatalf("%s : %s", instanceType, err.Error()) - } +func testListBucketsHandler(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, + credentials credential, t *testing.T) { testCases := []struct { bucketName string @@ -370,4 +323,17 @@ func testListBucketsHandler(obj ObjectLayer, instanceType string, t TestErrHandl t.Errorf("Test %d: %s: Expected the response status to be `%d`, but instead found `%d`", i+1, instanceType, testCase.expectedRespStatus, rec.Code) } } + + // Test for Anonymous/unsigned http request. + // ListBucketsHandler doesn't support bucket policies, setting the policies shouldn't make a difference. + anonReq, err := newTestRequest("GET", getListBucketURL(""), 0, nil) + + if err != nil { + t.Fatalf("Minio %s: Failed to create an anonymous request.", instanceType) + } + + // ExecObjectLayerAPIAnonTest - Calls the HTTP API handler using the anonymous request, validates the ErrAccessDeniedResponse, + // sets the bucket policy using the policy statement generated from `getWriteOnlyObjectStatement` so that the + // unsigned request goes through and its validated again. + ExecObjectLayerAPIAnonTest(t, "ListBucketsHandler", "", "", instanceType, apiRouter, anonReq, getWriteOnlyObjectStatement) } diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 63c06d66a..dee6e77f5 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -1410,6 +1410,7 @@ func ExecObjectLayerAPIAnonTest(t *testing.T, testName, bucketName, objectName, if err != nil { failTest(err.Error()) } + // creating 2 read closer (to set as request body) from the body content. readerOne := ioutil.NopCloser(bytes.NewBuffer(buf)) readerTwo := ioutil.NopCloser(bytes.NewBuffer(buf)) @@ -1428,16 +1429,18 @@ func ExecObjectLayerAPIAnonTest(t *testing.T, testName, bucketName, objectName, // expected error response in bytes when objectLayer is not initialized, or set to `nil`. expectedErrResponse := encodeResponse(getAPIErrorResponse(getAPIError(ErrAccessDenied), getGetObjectURL("", bucketName, objectName))) - // read the response body. - actualContent, err := ioutil.ReadAll(rec.Body) - if err != nil { - failTest(fmt.Sprintf("Failed parsing response body: %v", err)) + // HEAD HTTTP request doesn't contain response body. + if anonReq.Method != "HEAD" { + // read the response body. + actualContent, err := ioutil.ReadAll(rec.Body) + if err != nil { + failTest(fmt.Sprintf("Failed parsing response body: %v", err)) + } + // verify whether actual error response (from the response body), matches the expected error response. + if !bytes.Equal(expectedErrResponse, actualContent) { + failTest("error response content differs from expected value") + } } - // verify whether actual error response (from the response body), matches the expected error response. - if !bytes.Equal(expectedErrResponse, actualContent) { - failTest("Object content differs from expected value.") - } - // Set write only policy on bucket to allow anonymous HTTP request for the operation under test. // request to go through. policy := bucketPolicy{ @@ -1457,8 +1460,8 @@ func ExecObjectLayerAPIAnonTest(t *testing.T, testName, bucketName, objectName, // expectedHTTPStatus returns 204 (http.StatusNoContent) on success. if testName == "TestAPIDeleteObjectHandler" { expectedHTTPStatus = http.StatusNoContent - } else if strings.Contains(testName, "BucketPolicyHandler") { - // BucketPolicyHandler's doesn't support anonymous request, policy changes should allow unsigned requests. + } else if strings.Contains(testName, "BucketPolicyHandler") || testName == "ListBucketsHandler" { + // BucketPolicyHandlers and `ListBucketsHandler` doesn't support anonymous request, policy changes should allow unsigned requests. expectedHTTPStatus = http.StatusForbidden } else { // other API handlers return 200OK on success.