From 107e077ec0fe45d34c2f88adddae2745121a839a Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 22 Apr 2015 16:28:13 -0700 Subject: [PATCH 1/5] Implement x-amz-acl handling --- pkg/api/acl.go | 72 +++++++++++++++++++++++++++++++++ pkg/api/api_bucket_handlers.go | 47 ++++++++++++++++----- pkg/api/api_generic_handlers.go | 11 ----- pkg/api/api_object_handlers.go | 30 ++++++++++---- pkg/api/api_response.go | 12 ++++++ pkg/api/contenttype.go | 12 ++++-- pkg/api/errors.go | 13 +++++- pkg/api/headers.go | 5 ++- pkg/api/utils.go | 14 +++++++ 9 files changed, 182 insertions(+), 34 deletions(-) create mode 100644 pkg/api/acl.go create mode 100644 pkg/api/utils.go diff --git a/pkg/api/acl.go b/pkg/api/acl.go new file mode 100644 index 000000000..da21d36dc --- /dev/null +++ b/pkg/api/acl.go @@ -0,0 +1,72 @@ +/* + * Minimalist Object Storage, (C) 2015 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package api + +import ( + "net/http" + "strings" +) + +// Please read for more information - http://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl +// +// Here We are only supporting 'acl's through request headers not through their request body +// http://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#setting-acls + +// Minio only supports three types for now i.e 'private, public-read, public-read-write' +type ACLType int + +const ( + unsupportedACLType ACLType = iota + privateACLType + publicReadACLType + publicReadWriteACLType +) + +// Get acl type requested from 'x-amz-acl' header +func getACLType(req *http.Request) ACLType { + aclHeader := req.Header.Get("x-amz-acl") + switch { + case strings.HasPrefix(aclHeader, "private"): + return privateACLType + case strings.HasPrefix(aclHeader, "public-read"): + return publicReadACLType + case strings.HasPrefix(aclHeader, "public-read-write"): + return publicReadWriteACLType + default: + return unsupportedACLType + } +} + +// ACL type to human readable string +func getACLTypeString(acl ACLType) string { + switch acl { + case privateACLType: + { + return "private" + } + case publicReadACLType: + { + return "public-read" + } + case publicReadWriteACLType: + { + return "public-read-write" + } + default: + return "" + } +} diff --git a/pkg/api/api_bucket_handlers.go b/pkg/api/api_bucket_handlers.go index 26a9b29e5..0137bbbbd 100644 --- a/pkg/api/api_bucket_handlers.go +++ b/pkg/api/api_bucket_handlers.go @@ -32,14 +32,19 @@ import ( // criteria to return a subset of the objects in a bucket. // func (server *minioAPI) listObjectsHandler(w http.ResponseWriter, req *http.Request) { - vars := mux.Vars(req) - bucket := vars["bucket"] + acceptsContentType := getContentType(req) + if acceptsContentType == unknownContentType { + writeErrorResponse(w, req, NotAcceptable, acceptsContentType, req.URL.Path) + return + } resources := getBucketResources(req.URL.Query()) if resources.Maxkeys == 0 { resources.Maxkeys = maxObjectList } - acceptsContentType := getContentType(req) + + vars := mux.Vars(req) + bucket := vars["bucket"] objects, resources, err := server.driver.ListObjects(bucket, resources) switch err.(type) { case nil: // success @@ -49,8 +54,8 @@ func (server *minioAPI) listObjectsHandler(w http.ResponseWriter, req *http.Requ w.WriteHeader(http.StatusOK) // write body response := generateObjectsListResult(bucket, objects, resources) - encodedResponse := encodeResponse(response, acceptsContentType) - w.Write(encodedResponse) + encodedSuccessResponse := encodeSuccessResponse(response, acceptsContentType) + w.Write(encodedSuccessResponse) } case drivers.BucketNotFound: { @@ -78,6 +83,11 @@ func (server *minioAPI) listObjectsHandler(w http.ResponseWriter, req *http.Requ // owned by the authenticated sender of the request. func (server *minioAPI) listBucketsHandler(w http.ResponseWriter, req *http.Request) { acceptsContentType := getContentType(req) + if acceptsContentType == unknownContentType { + writeErrorResponse(w, req, NotAcceptable, acceptsContentType, req.URL.Path) + return + } + buckets, err := server.driver.ListBuckets() // cannot fallthrough in (type) switch :( switch err := err.(type) { @@ -88,8 +98,8 @@ func (server *minioAPI) listBucketsHandler(w http.ResponseWriter, req *http.Requ setCommonHeaders(w, getContentTypeString(acceptsContentType)) w.WriteHeader(http.StatusOK) // write response - encodedResponse := encodeResponse(response, acceptsContentType) - w.Write(encodedResponse) + encodedSuccessResponse := encodeSuccessResponse(response, acceptsContentType) + w.Write(encodedSuccessResponse) } default: { @@ -103,11 +113,22 @@ func (server *minioAPI) listBucketsHandler(w http.ResponseWriter, req *http.Requ // ---------- // This implementation of the PUT operation creates a new bucket for authenticated request func (server *minioAPI) putBucketHandler(w http.ResponseWriter, req *http.Request) { + acceptsContentType := getContentType(req) + if acceptsContentType == unknownContentType { + writeErrorResponse(w, req, NotAcceptable, acceptsContentType, req.URL.Path) + return + } + + // read from 'x-amz-acl' + aclType := getACLType(req) + if aclType == unsupportedACLType { + writeErrorResponse(w, req, NotImplemented, acceptsContentType, req.URL.Path) + return + } + vars := mux.Vars(req) bucket := vars["bucket"] err := server.driver.CreateBucket(bucket) - - acceptsContentType := getContentType(req) switch err.(type) { case nil: { @@ -138,10 +159,14 @@ func (server *minioAPI) putBucketHandler(w http.ResponseWriter, req *http.Reques // have permission to access it. Otherwise, the operation might // return responses such as 404 Not Found and 403 Forbidden. func (server *minioAPI) headBucketHandler(w http.ResponseWriter, req *http.Request) { - vars := mux.Vars(req) - bucket := vars["bucket"] acceptsContentType := getContentType(req) + if acceptsContentType == unknownContentType { + writeErrorResponse(w, req, NotAcceptable, acceptsContentType, req.URL.Path) + return + } + vars := mux.Vars(req) + bucket := vars["bucket"] _, err := server.driver.GetBucketMetadata(bucket) switch err.(type) { case nil: diff --git a/pkg/api/api_generic_handlers.go b/pkg/api/api_generic_handlers.go index a9308c1d3..d60decf4e 100644 --- a/pkg/api/api_generic_handlers.go +++ b/pkg/api/api_generic_handlers.go @@ -142,14 +142,3 @@ func ignoreUnImplementedObjectResources(req *http.Request) bool { } return false } - -func writeErrorResponse(w http.ResponseWriter, req *http.Request, errorType int, acceptsContentType contentType, resource string) { - error := getErrorCode(errorType) - errorResponse := getErrorResponse(error, resource) - // set headers - setCommonHeaders(w, getContentTypeString(acceptsContentType)) - w.WriteHeader(error.HTTPStatusCode) - // write body - encodedErrorResponse := encodeErrorResponse(errorResponse, acceptsContentType) - w.Write(encodedErrorResponse) -} diff --git a/pkg/api/api_object_handlers.go b/pkg/api/api_object_handlers.go index 28a6b24b4..152a645ac 100644 --- a/pkg/api/api_object_handlers.go +++ b/pkg/api/api_object_handlers.go @@ -30,12 +30,16 @@ import ( // This implementation of the GET operation retrieves object. To use GET, // you must have READ access to the object. func (server *minioAPI) getObjectHandler(w http.ResponseWriter, req *http.Request) { - var object, bucket string acceptsContentType := getContentType(req) + if acceptsContentType == unknownContentType { + writeErrorResponse(w, req, NotAcceptable, acceptsContentType, req.URL.Path) + return + } + + var object, bucket string vars := mux.Vars(req) bucket = vars["bucket"] object = vars["object"] - metadata, err := server.driver.GetObjectMetadata(bucket, object, "") switch err := err.(type) { case nil: // success @@ -95,12 +99,16 @@ func (server *minioAPI) getObjectHandler(w http.ResponseWriter, req *http.Reques // ----------- // The HEAD operation retrieves metadata from an object without returning the object itself. func (server *minioAPI) headObjectHandler(w http.ResponseWriter, req *http.Request) { - var object, bucket string acceptsContentType := getContentType(req) + if acceptsContentType == unknownContentType { + writeErrorResponse(w, req, NotAcceptable, acceptsContentType, req.URL.Path) + return + } + + var object, bucket string vars := mux.Vars(req) bucket = vars["bucket"] object = vars["object"] - metadata, err := server.driver.GetObjectMetadata(bucket, object, "") switch err := err.(type) { case nil: @@ -128,14 +136,22 @@ func (server *minioAPI) headObjectHandler(w http.ResponseWriter, req *http.Reque // ---------- // This implementation of the PUT operation adds an object to a bucket. func (server *minioAPI) putObjectHandler(w http.ResponseWriter, req *http.Request) { + acceptsContentType := getContentType(req) + if acceptsContentType == unknownContentType { + writeErrorResponse(w, req, NotAcceptable, acceptsContentType, req.URL.Path) + return + } + var object, bucket string vars := mux.Vars(req) - acceptsContentType := getContentType(req) bucket = vars["bucket"] object = vars["object"] - - // get Content-MD5 sent by client + // get Content-MD5 sent by client and verify if valid md5 := req.Header.Get("Content-MD5") + if !isValidMD5(md5) { + writeErrorResponse(w, req, InvalidDigest, acceptsContentType, req.URL.Path) + return + } err := server.driver.CreateObject(bucket, object, "", md5, req.Body) switch err := err.(type) { case nil: diff --git a/pkg/api/api_response.go b/pkg/api/api_response.go index 00a17bae2..a7b2c9065 100644 --- a/pkg/api/api_response.go +++ b/pkg/api/api_response.go @@ -17,6 +17,7 @@ package api import ( + "net/http" "sort" "github.com/minio-io/minio/pkg/storage/drivers" @@ -107,3 +108,14 @@ func generateObjectsListResult(bucket string, objects []drivers.ObjectMetadata, data.CommonPrefixes = prefixes return data } + +func writeErrorResponse(w http.ResponseWriter, req *http.Request, errorType int, acceptsContentType contentType, resource string) { + error := getErrorCode(errorType) + errorResponse := getErrorResponse(error, resource) + // set headers + setCommonHeaders(w, getContentTypeString(acceptsContentType)) + w.WriteHeader(error.HTTPStatusCode) + // write body + encodedErrorResponse := encodeErrorResponse(errorResponse, acceptsContentType) + w.Write(encodedErrorResponse) +} diff --git a/pkg/api/contenttype.go b/pkg/api/contenttype.go index 44921f46e..8cd164d40 100644 --- a/pkg/api/contenttype.go +++ b/pkg/api/contenttype.go @@ -24,7 +24,8 @@ import ( type contentType int const ( - xmlContentType contentType = iota + unknownContentType contentType = iota + xmlContentType jsonContentType ) @@ -34,8 +35,10 @@ func getContentType(req *http.Request) contentType { switch { case strings.HasPrefix(acceptHeader, "application/json"): return jsonContentType - default: + case strings.HasPrefix(acceptHeader, "application/xml"): return xmlContentType + default: + return unknownContentType } } @@ -44,9 +47,12 @@ func getContentTypeString(content contentType) string { switch content { case jsonContentType: { - return "application/json" } + case xmlContentType: + { + return "application/xml" + } default: { return "application/xml" diff --git a/pkg/api/errors.go b/pkg/api/errors.go index dcd7e2b8f..d316be67e 100644 --- a/pkg/api/errors.go +++ b/pkg/api/errors.go @@ -38,7 +38,7 @@ type ErrorResponse struct { HostID string } -// Error codes, non exhaustive list +// Error codes, non exhaustive list - http://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html const ( AccessDenied = iota BadDigest @@ -63,6 +63,11 @@ const ( TooManyBuckets ) +// Error codes, non exhaustive list - standard HTTP errors +const ( + NotAcceptable = iota + 21 +) + // Error code to Error structure map var errorCodeResponse = map[int]Error{ AccessDenied: { @@ -170,6 +175,12 @@ var errorCodeResponse = map[int]Error{ Description: "You have attempted to create more buckets than allowed.", HTTPStatusCode: http.StatusBadRequest, }, + NotAcceptable: { + Code: "NotAcceptable", + Description: `The requested resource is only capable of generating content + not acceptable according to the Accept headers sent in the request.`, + HTTPStatusCode: http.StatusNotAcceptable, + }, } // errorCodeError provides errorCode to Error. It returns empty if the code provided is unknown diff --git a/pkg/api/headers.go b/pkg/api/headers.go index ac9725f59..3e284f7dd 100644 --- a/pkg/api/headers.go +++ b/pkg/api/headers.go @@ -52,6 +52,9 @@ func encodeErrorResponse(response interface{}, acceptsType contentType) []byte { encoder = xml.NewEncoder(&bytesBuffer) case jsonContentType: encoder = json.NewEncoder(&bytesBuffer) + // by default even if unknown Accept header received handle it by sending XML contenttype response + default: + encoder = xml.NewEncoder(&bytesBuffer) } encoder.Encode(response) return bytesBuffer.Bytes() @@ -77,7 +80,7 @@ func setRangeObjectHeaders(w http.ResponseWriter, metadata drivers.ObjectMetadat w.Header().Set("Content-Range", contentRange.getContentRange()) } -func encodeResponse(response interface{}, acceptsType contentType) []byte { +func encodeSuccessResponse(response interface{}, acceptsType contentType) []byte { var encoder encoder var bytesBuffer bytes.Buffer switch acceptsType { diff --git a/pkg/api/utils.go b/pkg/api/utils.go new file mode 100644 index 000000000..33fc2352e --- /dev/null +++ b/pkg/api/utils.go @@ -0,0 +1,14 @@ +package api + +import ( + "encoding/base64" + "strings" +) + +func isValidMD5(md5 string) bool { + _, err := base64.StdEncoding.DecodeString(strings.TrimSpace(md5)) + if err != nil { + return false + } + return true +} From 2c1455af1bb90035b69a7d8d0f9763040056bf1e Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 22 Apr 2015 16:28:23 -0700 Subject: [PATCH 2/5] Implement x-amz-acl tests --- pkg/api/api_test.go | 17 +++++++++++++++++ pkg/api/contenttype.go | 22 +++++++++++----------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 3afe326d2..7413c027f 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -453,6 +453,7 @@ func (s *MySuite) TestPutBucket(c *C) { typedDriver.On("CreateBucket", "bucket").Return(nil).Once() request, err := http.NewRequest("PUT", testServer.URL+"/bucket", bytes.NewBufferString("")) c.Assert(err, IsNil) + request.Header.Add("x-amz-acl", "private") client := http.Client{} response, err := client.Do(request) @@ -499,6 +500,7 @@ func (s *MySuite) TestPutObject(c *C) { typedDriver.On("CreateBucket", "bucket").Return(nil).Once() request, err := http.NewRequest("PUT", testServer.URL+"/bucket", bytes.NewBufferString("")) c.Assert(err, IsNil) + request.Header.Add("x-amz-acl", "private") client := http.Client{} response, err := client.Do(request) @@ -970,6 +972,8 @@ func (s *MySuite) TestPutBucketErrors(c *C) { typedDriver.On("CreateBucket", "foo").Return(drivers.BucketNameInvalid{}).Once() request, err := http.NewRequest("PUT", testServer.URL+"/foo", bytes.NewBufferString("")) c.Assert(err, IsNil) + request.Header.Add("x-amz-acl", "private") + response, err := client.Do(request) c.Assert(err, IsNil) verifyError(c, response, "InvalidBucketName", "The specified bucket is not valid.", http.StatusBadRequest) @@ -977,6 +981,8 @@ func (s *MySuite) TestPutBucketErrors(c *C) { typedDriver.On("CreateBucket", "foo").Return(drivers.BucketExists{}).Once() request, err = http.NewRequest("PUT", testServer.URL+"/foo", bytes.NewBufferString("")) c.Assert(err, IsNil) + request.Header.Add("x-amz-acl", "private") + response, err = client.Do(request) c.Assert(err, IsNil) verifyError(c, response, "BucketAlreadyExists", "The requested bucket name is not available.", http.StatusConflict) @@ -984,9 +990,20 @@ func (s *MySuite) TestPutBucketErrors(c *C) { typedDriver.On("CreateBucket", "foo").Return(drivers.BackendCorrupted{}).Once() request, err = http.NewRequest("PUT", testServer.URL+"/foo", bytes.NewBufferString("")) c.Assert(err, IsNil) + request.Header.Add("x-amz-acl", "private") + response, err = client.Do(request) c.Assert(err, IsNil) verifyError(c, response, "InternalError", "We encountered an internal error, please try again.", http.StatusInternalServerError) + + typedDriver.On("CreateBucket", "foo").Return(nil).Once() + request, err = http.NewRequest("PUT", testServer.URL+"/foo", bytes.NewBufferString("")) + c.Assert(err, IsNil) + request.Header.Add("x-amz-acl", "unknown") + + response, err = client.Do(request) + c.Assert(err, IsNil) + verifyError(c, response, "NotImplemented", "A header you provided implies functionality that is not implemented.", http.StatusNotImplemented) } func (s *MySuite) TestGetObjectErrors(c *C) { diff --git a/pkg/api/contenttype.go b/pkg/api/contenttype.go index 8cd164d40..75afd7e64 100644 --- a/pkg/api/contenttype.go +++ b/pkg/api/contenttype.go @@ -16,10 +16,7 @@ package api -import ( - "net/http" - "strings" -) +import "net/http" type contentType int @@ -32,14 +29,17 @@ const ( // Get content type requested from 'Accept' header func getContentType(req *http.Request) contentType { acceptHeader := req.Header.Get("Accept") - switch { - case strings.HasPrefix(acceptHeader, "application/json"): - return jsonContentType - case strings.HasPrefix(acceptHeader, "application/xml"): - return xmlContentType - default: - return unknownContentType + if acceptHeader != "" { + switch { + case acceptHeader == "application/json": + return jsonContentType + case acceptHeader == "application/xml": + return xmlContentType + default: + return unknownContentType + } } + return xmlContentType } // Content type to human readable string From c8713fd6502bf06b0bfcf2a43b0e7f983d8ccc4a Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 22 Apr 2015 18:19:20 -0700 Subject: [PATCH 3/5] Bring in full fledged acl support --- pkg/api/acl.go | 2 + pkg/api/api_bucket_handlers.go | 2 +- pkg/api/api_generic_handlers.go | 74 ++++++++++++++++++++------------- pkg/api/api_router.go | 2 +- pkg/api/api_test.go | 61 ++++++++++++++------------- 5 files changed, 81 insertions(+), 60 deletions(-) diff --git a/pkg/api/acl.go b/pkg/api/acl.go index da21d36dc..2fef7e679 100644 --- a/pkg/api/acl.go +++ b/pkg/api/acl.go @@ -27,6 +27,8 @@ import ( // http://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#setting-acls // Minio only supports three types for now i.e 'private, public-read, public-read-write' + +// ACLType - different acl types type ACLType int const ( diff --git a/pkg/api/api_bucket_handlers.go b/pkg/api/api_bucket_handlers.go index 0137bbbbd..e17031c55 100644 --- a/pkg/api/api_bucket_handlers.go +++ b/pkg/api/api_bucket_handlers.go @@ -128,7 +128,7 @@ func (server *minioAPI) putBucketHandler(w http.ResponseWriter, req *http.Reques vars := mux.Vars(req) bucket := vars["bucket"] - err := server.driver.CreateBucket(bucket) + err := server.driver.CreateBucket(bucket, getACLTypeString(aclType)) switch err.(type) { case nil: { diff --git a/pkg/api/api_generic_handlers.go b/pkg/api/api_generic_handlers.go index d60decf4e..a4c5f3d23 100644 --- a/pkg/api/api_generic_handlers.go +++ b/pkg/api/api_generic_handlers.go @@ -20,11 +20,14 @@ import ( "net/http" "strings" + "github.com/gorilla/mux" "github.com/minio-io/minio/pkg/api/config" + "github.com/minio-io/minio/pkg/storage/drivers" ) type vHandler struct { conf config.Config + driver drivers.Driver handler http.Handler } @@ -47,43 +50,56 @@ func stripAccessKey(r *http.Request) string { // Validate handler is wrapper handler used for API request validation with authorization header. // Current authorization layer supports S3's standard HMAC based signature request. -func validateHandler(conf config.Config, h http.Handler) http.Handler { - return vHandler{conf, h} +func validateHandler(conf config.Config, driver drivers.Driver, h http.Handler) http.Handler { + return vHandler{ + conf: conf, + driver: driver, + handler: h, + } } // Validate handler ServeHTTP() wrapper func (h vHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { accessKey := stripAccessKey(r) acceptsContentType := getContentType(r) - if accessKey != "" { + if acceptsContentType == unknownContentType { + writeErrorResponse(w, r, NotAcceptable, acceptsContentType, r.URL.Path) + return + } + // verify for if bucket is private or public + vars := mux.Vars(r) + bucket, ok := vars["bucket"] + if ok { + bucketMetadata, err := h.driver.GetBucketMetadata(bucket) + if err != nil { + writeErrorResponse(w, r, AccessDenied, acceptsContentType, r.URL.Path) + return + } + if accessKey == "" && bucketMetadata.ACL.IsPrivate() { + writeErrorResponse(w, r, AccessDenied, acceptsContentType, r.URL.Path) + return + } + } + + switch true { + case accessKey != "": if err := h.conf.ReadConfig(); err != nil { - error := getErrorCode(InternalError) - errorResponse := getErrorResponse(error, "") - setCommonHeaders(w, getContentTypeString(acceptsContentType)) - w.WriteHeader(error.HTTPStatusCode) - w.Write(encodeErrorResponse(errorResponse, acceptsContentType)) - } else { - user, ok := h.conf.Users[accessKey] - if ok == false { - error := getErrorCode(AccessDenied) - errorResponse := getErrorResponse(error, "") - setCommonHeaders(w, getContentTypeString(acceptsContentType)) - w.WriteHeader(error.HTTPStatusCode) - w.Write(encodeErrorResponse(errorResponse, acceptsContentType)) - } else { - ok, _ = ValidateRequest(user, r) - if ok { - h.handler.ServeHTTP(w, r) - } else { - error := getErrorCode(AccessDenied) - errorResponse := getErrorResponse(error, "") - setCommonHeaders(w, getContentTypeString(acceptsContentType)) - w.WriteHeader(error.HTTPStatusCode) - w.Write(encodeErrorResponse(errorResponse, acceptsContentType)) - } - } + writeErrorResponse(w, r, InternalError, acceptsContentType, r.URL.Path) + return } - } else { + user, ok := h.conf.Users[accessKey] + if !ok { + writeErrorResponse(w, r, AccessDenied, acceptsContentType, r.URL.Path) + return + } + ok, _ = ValidateRequest(user, r) + if !ok { + writeErrorResponse(w, r, AccessDenied, acceptsContentType, r.URL.Path) + return + } + // Success + h.handler.ServeHTTP(w, r) + default: // Control reaches when no access key is found, ideally we would // like to throw back `403`. But for now with our tests lacking // this functionality it is better for us to be serving anonymous diff --git a/pkg/api/api_router.go b/pkg/api/api_router.go index 0e5813a52..d4b2d65d7 100644 --- a/pkg/api/api_router.go +++ b/pkg/api/api_router.go @@ -89,5 +89,5 @@ func HTTPHandler(domain string, driver drivers.Driver) http.Handler { log.Fatal(iodine.New(err, map[string]string{"domain": domain})) } - return validateHandler(conf, ignoreResourcesHandler(mux)) + return validateHandler(conf, api.driver, ignoreResourcesHandler(mux)) } diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 7413c027f..474296c43 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -142,7 +142,7 @@ func (s *MySuite) TestEmptyObject(c *C) { Md5: "d41d8cd98f00b204e9800998ecf8427e", Size: 0, } - typedDriver.On("CreateBucket", "bucket").Return(nil).Once() + typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything).Return(nil).Once() typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(metadata, nil).Once() typedDriver.On("GetObject", mock.Anything, "bucket", "object").Return(int64(0), nil).Once() @@ -152,7 +152,7 @@ func (s *MySuite) TestEmptyObject(c *C) { defer testServer.Close() buffer := bytes.NewBufferString("") - driver.CreateBucket("bucket") + driver.CreateBucket("bucket", "private") driver.CreateObject("bucket", "object", "", "", buffer) response, err := http.Get(testServer.URL + "/bucket/object") @@ -181,14 +181,14 @@ func (s *MySuite) TestBucket(c *C) { Name: "bucket", Created: time.Now(), } - typedDriver.On("CreateBucket", "bucket").Return(nil).Once() + typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() typedDriver.On("GetBucketMetadata", "bucket").Return(metadata, nil).Twice() httpHandler := api.HTTPHandler("", driver) testServer := httptest.NewServer(httpHandler) defer testServer.Close() - driver.CreateBucket("bucket") + driver.CreateBucket("bucket", "private") response, err := http.Head(testServer.URL + "/bucket") c.Assert(err, IsNil) @@ -212,7 +212,7 @@ func (s *MySuite) TestObject(c *C) { Md5: "5eb63bbbe01eeed093cb22bb8f5acdc3", Size: 11, } - typedDriver.On("CreateBucket", "bucket").Return(nil).Once() + typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything).Return(nil).Once() typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(metadata, nil).Twice() typedDriver.SetGetObjectWriter("bucket", "object", []byte("hello world")) @@ -223,7 +223,7 @@ func (s *MySuite) TestObject(c *C) { defer testServer.Close() buffer := bytes.NewBufferString("hello world") - driver.CreateBucket("bucket") + driver.CreateBucket("bucket", "private") driver.CreateObject("bucket", "object", "", "", buffer) response, err := http.Get(testServer.URL + "/bucket/object") @@ -280,8 +280,8 @@ func (s *MySuite) TestMultipleObjects(c *C) { buffer2 := bytes.NewBufferString("hello two") buffer3 := bytes.NewBufferString("hello three") - typedDriver.On("CreateBucket", "bucket").Return(nil).Once() - driver.CreateBucket("bucket") + typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() + driver.CreateBucket("bucket", "private") typedDriver.On("CreateObject", "bucket", "object1", "", "", mock.Anything).Return(nil).Once() driver.CreateObject("bucket", "object1", "", "", buffer1) typedDriver.On("CreateObject", "bucket", "object2", "", "", mock.Anything).Return(nil).Once() @@ -397,8 +397,8 @@ func (s *MySuite) TestHeader(c *C) { testServer := httptest.NewServer(httpHandler) defer testServer.Close() - typedDriver.On("CreateBucket", "bucket").Return(nil).Once() - driver.CreateBucket("bucket") + typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() + driver.CreateBucket("bucket", "private") typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(drivers.ObjectMetadata{}, drivers.ObjectNotFound{}).Once() response, err := http.Get(testServer.URL + "/bucket/object") @@ -450,7 +450,7 @@ func (s *MySuite) TestPutBucket(c *C) { c.Assert(len(buckets), Equals, 0) c.Assert(err, IsNil) - typedDriver.On("CreateBucket", "bucket").Return(nil).Once() + typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() request, err := http.NewRequest("PUT", testServer.URL+"/bucket", bytes.NewBufferString("")) c.Assert(err, IsNil) request.Header.Add("x-amz-acl", "private") @@ -497,7 +497,7 @@ func (s *MySuite) TestPutObject(c *C) { date1 := time.Now().Add(-time.Second) // Put Bucket before - Put Object into a bucket - typedDriver.On("CreateBucket", "bucket").Return(nil).Once() + typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() request, err := http.NewRequest("PUT", testServer.URL+"/bucket", bytes.NewBufferString("")) c.Assert(err, IsNil) request.Header.Add("x-amz-acl", "private") @@ -575,8 +575,9 @@ func (s *MySuite) TestListBuckets(c *C) { c.Assert(err, IsNil) c.Assert(len(listResponse.Buckets.Bucket), Equals, 0) - typedDriver.On("CreateBucket", "foo").Return(nil).Once() - driver.CreateBucket("foo") + typedDriver.On("CreateBucket", "foo", "private").Return(nil).Once() + err = driver.CreateBucket("foo", "private") + c.Assert(err, IsNil) bucketMetadata := []drivers.BucketMetadata{ {Name: "foo", Created: time.Now()}, @@ -592,8 +593,9 @@ func (s *MySuite) TestListBuckets(c *C) { c.Assert(len(listResponse.Buckets.Bucket), Equals, 1) c.Assert(listResponse.Buckets.Bucket[0].Name, Equals, "foo") - typedDriver.On("CreateBucket", "bar").Return(nil).Once() - driver.CreateBucket("bar") + typedDriver.On("CreateBucket", "bar", "private").Return(nil).Once() + err = driver.CreateBucket("bar", "private") + c.Assert(err, IsNil) bucketMetadata = []drivers.BucketMetadata{ {Name: "bar", Created: time.Now()}, @@ -689,8 +691,8 @@ func (s *MySuite) TestXMLNameNotInBucketListJson(c *C) { testServer := httptest.NewServer(httpHandler) defer testServer.Close() - typedDriver.On("CreateBucket", "foo").Return(nil).Once() - err := driver.CreateBucket("foo") + typedDriver.On("CreateBucket", "foo", "private").Return(nil).Once() + err := driver.CreateBucket("foo", "private") c.Assert(err, IsNil) typedDriver.On("ListBuckets").Return([]drivers.BucketMetadata{{Name: "foo", Created: time.Now()}}, nil) @@ -723,8 +725,8 @@ func (s *MySuite) TestXMLNameNotInObjectListJson(c *C) { testServer := httptest.NewServer(httpHandler) defer testServer.Close() - typedDriver.On("CreateBucket", "foo").Return(nil).Once() - err := driver.CreateBucket("foo") + typedDriver.On("CreateBucket", "foo", "private").Return(nil).Once() + err := driver.CreateBucket("foo", "private") c.Assert(err, IsNil) typedDriver.On("ListObjects", "foo", mock.Anything).Return([]drivers.ObjectMetadata{}, drivers.BucketResourcesMetadata{}, nil).Once() @@ -758,8 +760,8 @@ func (s *MySuite) TestContentTypePersists(c *C) { testServer := httptest.NewServer(httpHandler) defer testServer.Close() - typedDriver.On("CreateBucket", "bucket").Return(nil).Once() - err := driver.CreateBucket("bucket") + typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() + err := driver.CreateBucket("bucket", "private") c.Assert(err, IsNil) client := http.Client{} @@ -849,10 +851,11 @@ func (s *MySuite) TestPartialContent(c *C) { Size: 11, } - typedDriver.On("CreateBucket", "foo").Return(nil).Once() - + typedDriver.On("CreateBucket", "foo", "private").Return(nil).Once() typedDriver.On("CreateObject", "foo", "bar", "", "", mock.Anything).Return(nil).Once() - driver.CreateBucket("foo") + err := driver.CreateBucket("foo", "private") + c.Assert(err, IsNil) + driver.CreateObject("foo", "bar", "", "", bytes.NewBufferString("hello world")) // prepare for GET on range request @@ -969,7 +972,7 @@ func (s *MySuite) TestPutBucketErrors(c *C) { defer testServer.Close() client := http.Client{} - typedDriver.On("CreateBucket", "foo").Return(drivers.BucketNameInvalid{}).Once() + typedDriver.On("CreateBucket", "foo", "private").Return(drivers.BucketNameInvalid{}).Once() request, err := http.NewRequest("PUT", testServer.URL+"/foo", bytes.NewBufferString("")) c.Assert(err, IsNil) request.Header.Add("x-amz-acl", "private") @@ -978,7 +981,7 @@ func (s *MySuite) TestPutBucketErrors(c *C) { c.Assert(err, IsNil) verifyError(c, response, "InvalidBucketName", "The specified bucket is not valid.", http.StatusBadRequest) - typedDriver.On("CreateBucket", "foo").Return(drivers.BucketExists{}).Once() + typedDriver.On("CreateBucket", "foo", "private").Return(drivers.BucketExists{}).Once() request, err = http.NewRequest("PUT", testServer.URL+"/foo", bytes.NewBufferString("")) c.Assert(err, IsNil) request.Header.Add("x-amz-acl", "private") @@ -987,7 +990,7 @@ func (s *MySuite) TestPutBucketErrors(c *C) { c.Assert(err, IsNil) verifyError(c, response, "BucketAlreadyExists", "The requested bucket name is not available.", http.StatusConflict) - typedDriver.On("CreateBucket", "foo").Return(drivers.BackendCorrupted{}).Once() + typedDriver.On("CreateBucket", "foo", "private").Return(drivers.BackendCorrupted{}).Once() request, err = http.NewRequest("PUT", testServer.URL+"/foo", bytes.NewBufferString("")) c.Assert(err, IsNil) request.Header.Add("x-amz-acl", "private") @@ -996,7 +999,7 @@ func (s *MySuite) TestPutBucketErrors(c *C) { c.Assert(err, IsNil) verifyError(c, response, "InternalError", "We encountered an internal error, please try again.", http.StatusInternalServerError) - typedDriver.On("CreateBucket", "foo").Return(nil).Once() + typedDriver.On("CreateBucket", "foo", "unknown").Return(nil).Once() request, err = http.NewRequest("PUT", testServer.URL+"/foo", bytes.NewBufferString("")) c.Assert(err, IsNil) request.Header.Add("x-amz-acl", "unknown") From 1c0ff2c7581fcfeb5329c7c86da66e9ef553c16c Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 22 Apr 2015 18:19:53 -0700 Subject: [PATCH 4/5] ACL driver/storage layer support --- pkg/api/api_generic_handlers.go | 4 ++ pkg/storage/donut/donut.go | 6 ++- pkg/storage/donut/donut_bucket.go | 7 ++- pkg/storage/donut/donut_public_interfaces.go | 2 +- pkg/storage/donut/donut_test.go | 24 +++++----- pkg/storage/donut/objectstorage.go | 5 +- pkg/storage/donut/objectstorage_internal.go | 7 +-- pkg/storage/drivers/api_testsuite.go | 36 +++++++------- pkg/storage/drivers/donut/donut.go | 16 +++++-- pkg/storage/drivers/driver.go | 49 +++++++++++++++++++- pkg/storage/drivers/errors.go | 11 +++++ pkg/storage/drivers/memory/memory.go | 12 +++-- pkg/storage/drivers/mocks/Driver.go | 4 +- 13 files changed, 133 insertions(+), 50 deletions(-) diff --git a/pkg/api/api_generic_handlers.go b/pkg/api/api_generic_handlers.go index a4c5f3d23..f26a313e6 100644 --- a/pkg/api/api_generic_handlers.go +++ b/pkg/api/api_generic_handlers.go @@ -79,6 +79,10 @@ func (h vHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { writeErrorResponse(w, r, AccessDenied, acceptsContentType, r.URL.Path) return } + if r.Method == "PUT" && bucketMetadata.ACL.IsPublicRead() { + writeErrorResponse(w, r, AccessDenied, acceptsContentType, r.URL.Path) + return + } } switch true { diff --git a/pkg/storage/donut/donut.go b/pkg/storage/donut/donut.go index e9888516f..165629d60 100644 --- a/pkg/storage/donut/donut.go +++ b/pkg/storage/donut/donut.go @@ -31,9 +31,13 @@ type donut struct { // config files used inside Donut const ( + // donut object metadata and config donutObjectMetadataConfig = "donutObjectMetadata.json" - objectMetadataConfig = "objectMetadata.json" donutConfig = "donutMetadata.json" + + // bucket, object metadata + bucketMetadataConfig = "bucketMetadata.json" + objectMetadataConfig = "objectMetadata.json" ) // attachDonutNode - wrapper function to instantiate a new node for associated donut diff --git a/pkg/storage/donut/donut_bucket.go b/pkg/storage/donut/donut_bucket.go index 5566cc83d..7733a6485 100644 --- a/pkg/storage/donut/donut_bucket.go +++ b/pkg/storage/donut/donut_bucket.go @@ -35,22 +35,27 @@ import ( // internal struct carrying bucket specific information type bucket struct { name string + acl string + time time.Time donutName string nodes map[string]Node objects map[string]Object } // NewBucket - instantiate a new bucket -func NewBucket(bucketName, donutName string, nodes map[string]Node) (Bucket, error) { +func NewBucket(bucketName, aclType, donutName string, nodes map[string]Node) (Bucket, error) { errParams := map[string]string{ "bucketName": bucketName, "donutName": donutName, + "aclType": aclType, } if strings.TrimSpace(bucketName) == "" || strings.TrimSpace(donutName) == "" { return nil, iodine.New(errors.New("invalid argument"), errParams) } b := bucket{} b.name = bucketName + b.acl = aclType + b.time = time.Now() b.donutName = donutName b.objects = make(map[string]Object) b.nodes = nodes diff --git a/pkg/storage/donut/donut_public_interfaces.go b/pkg/storage/donut/donut_public_interfaces.go index e2a2ff7a3..0aecef3b8 100644 --- a/pkg/storage/donut/donut_public_interfaces.go +++ b/pkg/storage/donut/donut_public_interfaces.go @@ -32,7 +32,7 @@ type ObjectStorage interface { GetBucketMetadata(bucket string) (map[string]string, error) SetBucketMetadata(bucket string, metadata map[string]string) error ListBuckets() ([]string, error) - MakeBucket(bucket string) error + MakeBucket(bucket, acl string) error // Bucket Operations ListObjects(bucket, prefix, marker, delim string, maxKeys int) (result []string, prefixes []string, isTruncated bool, err error) diff --git a/pkg/storage/donut/donut_test.go b/pkg/storage/donut/donut_test.go index 8c7195d48..3fb5f2e0d 100644 --- a/pkg/storage/donut/donut_test.go +++ b/pkg/storage/donut/donut_test.go @@ -75,10 +75,10 @@ func (s *MySuite) TestBucketWithoutNameFails(c *C) { donut, err := NewDonut("test", createTestNodeDiskMap(root)) c.Assert(err, IsNil) // fail to create new bucket without a name - err = donut.MakeBucket("") + err = donut.MakeBucket("", "private") c.Assert(err, Not(IsNil)) - err = donut.MakeBucket(" ") + err = donut.MakeBucket(" ", "private") c.Assert(err, Not(IsNil)) } @@ -90,7 +90,7 @@ func (s *MySuite) TestEmptyBucket(c *C) { donut, err := NewDonut("test", createTestNodeDiskMap(root)) c.Assert(err, IsNil) - c.Assert(donut.MakeBucket("foo"), IsNil) + c.Assert(donut.MakeBucket("foo", "private"), IsNil) // check if bucket is empty objects, _, istruncated, err := donut.ListObjects("foo", "", "", "", 1) c.Assert(err, IsNil) @@ -106,7 +106,7 @@ func (s *MySuite) TestMakeBucketAndList(c *C) { donut, err := NewDonut("test", createTestNodeDiskMap(root)) c.Assert(err, IsNil) // create bucket - err = donut.MakeBucket("foo") + err = donut.MakeBucket("foo", "private") c.Assert(err, IsNil) // check bucket exists @@ -122,10 +122,10 @@ func (s *MySuite) TestMakeBucketWithSameNameFails(c *C) { defer os.RemoveAll(root) donut, err := NewDonut("test", createTestNodeDiskMap(root)) c.Assert(err, IsNil) - err = donut.MakeBucket("foo") + err = donut.MakeBucket("foo", "private") c.Assert(err, IsNil) - err = donut.MakeBucket("foo") + err = donut.MakeBucket("foo", "private") c.Assert(err, Not(IsNil)) } @@ -137,17 +137,17 @@ func (s *MySuite) TestCreateMultipleBucketsAndList(c *C) { donut, err := NewDonut("test", createTestNodeDiskMap(root)) c.Assert(err, IsNil) // add a second bucket - err = donut.MakeBucket("foo") + err = donut.MakeBucket("foo", "private") c.Assert(err, IsNil) - err = donut.MakeBucket("bar") + err = donut.MakeBucket("bar", "private") c.Assert(err, IsNil) buckets, err := donut.ListBuckets() c.Assert(err, IsNil) c.Assert(buckets, DeepEquals, []string{"bar", "foo"}) - err = donut.MakeBucket("foobar") + err = donut.MakeBucket("foobar", "private") c.Assert(err, IsNil) buckets, err = donut.ListBuckets() @@ -185,7 +185,7 @@ func (s *MySuite) TestNewObjectMetadata(c *C) { expectedMd5Sum := hex.EncodeToString(hasher.Sum(nil)) reader := ioutil.NopCloser(bytes.NewReader([]byte(data))) - err = donut.MakeBucket("foo") + err = donut.MakeBucket("foo", "private") c.Assert(err, IsNil) err = donut.PutObject("foo", "obj", expectedMd5Sum, reader, metadata) @@ -222,7 +222,7 @@ func (s *MySuite) TestNewObjectCanBeWritten(c *C) { donut, err := NewDonut("test", createTestNodeDiskMap(root)) c.Assert(err, IsNil) - err = donut.MakeBucket("foo") + err = donut.MakeBucket("foo", "private") c.Assert(err, IsNil) metadata := make(map[string]string) @@ -263,7 +263,7 @@ func (s *MySuite) TestMultipleNewObjects(c *C) { donut, err := NewDonut("test", createTestNodeDiskMap(root)) c.Assert(err, IsNil) - c.Assert(donut.MakeBucket("foo"), IsNil) + c.Assert(donut.MakeBucket("foo", "private"), IsNil) one := ioutil.NopCloser(bytes.NewReader([]byte("one"))) err = donut.PutObject("foo", "obj1", "", one, nil) diff --git a/pkg/storage/donut/objectstorage.go b/pkg/storage/donut/objectstorage.go index 58ef79684..2fd37efed 100644 --- a/pkg/storage/donut/objectstorage.go +++ b/pkg/storage/donut/objectstorage.go @@ -28,11 +28,11 @@ import ( ) // MakeBucket - make a new bucket -func (d donut) MakeBucket(bucket string) error { +func (d donut) MakeBucket(bucket, acl string) error { if bucket == "" || strings.TrimSpace(bucket) == "" { return iodine.New(errors.New("invalid argument"), nil) } - return d.makeDonutBucket(bucket) + return d.makeDonutBucket(bucket, acl) } // GetBucketMetadata - get bucket metadata @@ -47,6 +47,7 @@ func (d donut) GetBucketMetadata(bucket string) (map[string]string, error) { metadata := make(map[string]string) metadata["name"] = bucket metadata["created"] = time.Now().Format(time.RFC3339Nano) // TODO get this, from whatever is written from SetBucketMetadata + metadata["acl"] = "private" return metadata, nil } diff --git a/pkg/storage/donut/objectstorage_internal.go b/pkg/storage/donut/objectstorage_internal.go index d679df606..acb0df821 100644 --- a/pkg/storage/donut/objectstorage_internal.go +++ b/pkg/storage/donut/objectstorage_internal.go @@ -25,7 +25,8 @@ import ( "github.com/minio-io/minio/pkg/iodine" ) -func (d donut) makeDonutBucket(bucketName string) error { +// TODO we have to store the acl's +func (d donut) makeDonutBucket(bucketName, acl string) error { err := d.getDonutBuckets() if err != nil { return iodine.New(err, nil) @@ -33,7 +34,7 @@ func (d donut) makeDonutBucket(bucketName string) error { if _, ok := d.buckets[bucketName]; ok { return iodine.New(errors.New("bucket exists"), nil) } - bucket, err := NewBucket(bucketName, d.name, d.nodes) + bucket, err := NewBucket(bucketName, acl, d.name, d.nodes) if err != nil { return iodine.New(err, nil) } @@ -74,7 +75,7 @@ func (d donut) getDonutBuckets() error { } bucketName := splitDir[0] // we dont need this NewBucket once we cache from makeDonutBucket() - bucket, err := NewBucket(bucketName, d.name, d.nodes) + bucket, err := NewBucket(bucketName, "private", d.name, d.nodes) if err != nil { return iodine.New(err, nil) } diff --git a/pkg/storage/drivers/api_testsuite.go b/pkg/storage/drivers/api_testsuite.go index aa561a76b..157a29f90 100644 --- a/pkg/storage/drivers/api_testsuite.go +++ b/pkg/storage/drivers/api_testsuite.go @@ -48,14 +48,14 @@ func APITestSuite(c *check.C, create func() Driver) { func testCreateBucket(c *check.C, create func() Driver) { drivers := create() - err := drivers.CreateBucket("bucket") + err := drivers.CreateBucket("bucket", "") c.Assert(err, check.IsNil) } func testMultipleObjectCreation(c *check.C, create func() Driver) { objects := make(map[string][]byte) drivers := create() - err := drivers.CreateBucket("bucket") + err := drivers.CreateBucket("bucket", "") c.Assert(err, check.IsNil) for i := 0; i < 10; i++ { randomPerm := rand.Perm(10) @@ -94,7 +94,7 @@ func testMultipleObjectCreation(c *check.C, create func() Driver) { func testPaging(c *check.C, create func() Driver) { drivers := create() - drivers.CreateBucket("bucket") + drivers.CreateBucket("bucket", "") resources := BucketResourcesMetadata{} objects, resources, err := drivers.ListObjects("bucket", resources) c.Assert(err, check.IsNil) @@ -198,7 +198,7 @@ func testPaging(c *check.C, create func() Driver) { func testObjectOverwriteFails(c *check.C, create func() Driver) { drivers := create() - drivers.CreateBucket("bucket") + drivers.CreateBucket("bucket", "") hasher1 := md5.New() hasher1.Write([]byte("one")) @@ -227,7 +227,7 @@ func testNonExistantBucketOperations(c *check.C, create func() Driver) { func testBucketMetadata(c *check.C, create func() Driver) { drivers := create() - err := drivers.CreateBucket("string") + err := drivers.CreateBucket("string", "") c.Assert(err, check.IsNil) metadata, err := drivers.GetBucketMetadata("string") @@ -237,15 +237,15 @@ func testBucketMetadata(c *check.C, create func() Driver) { func testBucketRecreateFails(c *check.C, create func() Driver) { drivers := create() - err := drivers.CreateBucket("string") + err := drivers.CreateBucket("string", "") c.Assert(err, check.IsNil) - err = drivers.CreateBucket("string") + err = drivers.CreateBucket("string", "") c.Assert(err, check.Not(check.IsNil)) } func testPutObjectInSubdir(c *check.C, create func() Driver) { drivers := create() - err := drivers.CreateBucket("bucket") + err := drivers.CreateBucket("bucket", "") c.Assert(err, check.IsNil) hasher := md5.New() @@ -270,7 +270,7 @@ func testListBuckets(c *check.C, create func() Driver) { c.Assert(len(buckets), check.Equals, 0) // add one and test exists - err = drivers.CreateBucket("bucket1") + err = drivers.CreateBucket("bucket1", "") c.Assert(err, check.IsNil) buckets, err = drivers.ListBuckets() @@ -278,7 +278,7 @@ func testListBuckets(c *check.C, create func() Driver) { c.Assert(err, check.IsNil) // add two and test exists - err = drivers.CreateBucket("bucket2") + err = drivers.CreateBucket("bucket2", "") c.Assert(err, check.IsNil) buckets, err = drivers.ListBuckets() @@ -286,7 +286,7 @@ func testListBuckets(c *check.C, create func() Driver) { c.Assert(err, check.IsNil) // add three and test exists + prefix - err = drivers.CreateBucket("bucket22") + err = drivers.CreateBucket("bucket22", "") buckets, err = drivers.ListBuckets() c.Assert(len(buckets), check.Equals, 3) @@ -299,8 +299,8 @@ func testListBucketsOrder(c *check.C, create func() Driver) { for i := 0; i < 10; i++ { drivers := create() // add one and test exists - drivers.CreateBucket("bucket1") - drivers.CreateBucket("bucket2") + drivers.CreateBucket("bucket1", "") + drivers.CreateBucket("bucket2", "") buckets, err := drivers.ListBuckets() c.Assert(len(buckets), check.Equals, 2) @@ -321,7 +321,7 @@ func testListObjectsTestsForNonExistantBucket(c *check.C, create func() Driver) func testNonExistantObjectInBucket(c *check.C, create func() Driver) { drivers := create() - err := drivers.CreateBucket("bucket") + err := drivers.CreateBucket("bucket", "") c.Assert(err, check.IsNil) var byteBuffer bytes.Buffer @@ -343,7 +343,7 @@ func testNonExistantObjectInBucket(c *check.C, create func() Driver) { func testGetDirectoryReturnsObjectNotFound(c *check.C, create func() Driver) { drivers := create() - err := drivers.CreateBucket("bucket") + err := drivers.CreateBucket("bucket", "") c.Assert(err, check.IsNil) err = drivers.CreateObject("bucket", "dir1/dir2/object", "", "", bytes.NewBufferString("hello world")) @@ -386,7 +386,7 @@ func testGetDirectoryReturnsObjectNotFound(c *check.C, create func() Driver) { func testDefaultContentType(c *check.C, create func() Driver) { drivers := create() - err := drivers.CreateBucket("bucket") + err := drivers.CreateBucket("bucket", "") c.Assert(err, check.IsNil) // test empty @@ -408,10 +408,9 @@ func testDefaultContentType(c *check.C, create func() Driver) { c.Assert(metadata.ContentType, check.Equals, "application/json") } -/* func testContentMd5Set(c *check.C, create func() Driver) { drivers := create() - err := drivers.CreateBucket("bucket") + err := drivers.CreateBucket("bucket", "") c.Assert(err, check.IsNil) // test md5 invalid @@ -420,4 +419,3 @@ func testContentMd5Set(c *check.C, create func() Driver) { err = drivers.CreateObject("bucket", "two", "", "NWJiZjVhNTIzMjhlNzQzOWFlNmU3MTlkZmU3MTIyMDA=", bytes.NewBufferString("one")) c.Assert(err, check.IsNil) } -*/ diff --git a/pkg/storage/drivers/donut/donut.go b/pkg/storage/drivers/donut/donut.go index e2ce2f223..8227f91e8 100644 --- a/pkg/storage/drivers/donut/donut.go +++ b/pkg/storage/drivers/donut/donut.go @@ -39,6 +39,7 @@ import ( // donutDriver - creates a new single disk drivers driver using donut type donutDriver struct { donut donut.Donut + path string } const ( @@ -82,6 +83,7 @@ func Start(path string) (chan<- string, <-chan error, drivers.Driver) { s := new(donutDriver) s.donut = donut + s.path = path go start(ctrlChannel, errorChannel, s) return ctrlChannel, errorChannel, s @@ -117,11 +119,14 @@ func (d donutDriver) ListBuckets() (results []drivers.BucketMetadata, err error) } // CreateBucket creates a new bucket -func (d donutDriver) CreateBucket(bucketName string) error { +func (d donutDriver) CreateBucket(bucketName, acl string) error { + if !drivers.IsValidBucketACL(acl) { + return iodine.New(drivers.InvalidACL{ACL: acl}, nil) + } if drivers.IsValidBucket(bucketName) && !strings.Contains(bucketName, ".") { - return d.donut.MakeBucket(bucketName) + return d.donut.MakeBucket(bucketName, acl) } - return iodine.New(errors.New("Invalid bucket"), map[string]string{"bucket": bucketName}) + return iodine.New(drivers.BucketNameInvalid{Bucket: bucketName}, nil) } // GetBucketMetadata retrieves an bucket's metadata @@ -137,9 +142,14 @@ func (d donutDriver) GetBucketMetadata(bucketName string) (drivers.BucketMetadat if err != nil { return drivers.BucketMetadata{}, iodine.New(err, nil) } + acl, ok := metadata["acl"] + if !ok { + return drivers.BucketMetadata{}, iodine.New(drivers.BackendCorrupted{Path: d.path}, nil) + } bucketMetadata := drivers.BucketMetadata{ Name: metadata["name"], Created: created, + ACL: drivers.BucketACL(acl), } return bucketMetadata, nil } diff --git a/pkg/storage/drivers/driver.go b/pkg/storage/drivers/driver.go index e6eb3571e..40792daaa 100644 --- a/pkg/storage/drivers/driver.go +++ b/pkg/storage/drivers/driver.go @@ -27,7 +27,7 @@ import ( type Driver interface { // Bucket Operations ListBuckets() ([]BucketMetadata, error) - CreateBucket(bucket string) error + CreateBucket(bucket, acl string) error GetBucketMetadata(bucket string) (BucketMetadata, error) // Object Operations @@ -38,10 +38,40 @@ type Driver interface { CreateObject(bucket string, key string, contentType string, md5sum string, data io.Reader) error } +// BucketACL - bucket level access control +type BucketACL string + +// different types of ACL's currently supported for buckets +const ( + BucketPrivate = BucketACL("private") + BucketPublicRead = BucketACL("public-read") + BucketPublicReadWrite = BucketACL("public-read-write") +) + +func (b BucketACL) String() string { + return string(b) +} + +// IsPrivate - is acl Private +func (b BucketACL) IsPrivate() bool { + return b == BucketACL("private") +} + +// IsPublicRead - is acl PublicRead +func (b BucketACL) IsPublicRead() bool { + return b == BucketACL("public-read") +} + +// IsPublicReadWrite - is acl PublicReadWrite +func (b BucketACL) IsPublicReadWrite() bool { + return b == BucketACL("public-read-write") +} + // BucketMetadata - name and create date type BucketMetadata struct { Name string Created time.Time + ACL BucketACL } // ObjectMetadata - object key and its relevant metadata @@ -98,6 +128,23 @@ func GetMode(resources BucketResourcesMetadata) FilterMode { return f } +// IsValidBucketACL - is provided acl string supported +func IsValidBucketACL(acl string) bool { + switch acl { + case "private": + fallthrough + case "public-read": + fallthrough + case "public-read-write": + return true + case "": + // by default its "private" + return true + default: + return false + } +} + // IsDelimiterPrefixSet Delimiter and Prefix set func (b BucketResourcesMetadata) IsDelimiterPrefixSet() bool { return b.Mode == DelimiterPrefixMode diff --git a/pkg/storage/drivers/errors.go b/pkg/storage/drivers/errors.go index fa0a51031..bcd3a2cd3 100644 --- a/pkg/storage/drivers/errors.go +++ b/pkg/storage/drivers/errors.go @@ -54,6 +54,17 @@ type DigestError struct { Md5 string } +/// ACL related errors + +// InvalidACL - acl invalid +type InvalidACL struct { + ACL string +} + +func (e InvalidACL) Error() string { + return "Requested ACL is " + e.ACL + " invalid" +} + /// Bucket related errors // BucketNameInvalid - bucketname provided is invalid diff --git a/pkg/storage/drivers/memory/memory.go b/pkg/storage/drivers/memory/memory.go index 9545521e3..62dcb365f 100644 --- a/pkg/storage/drivers/memory/memory.go +++ b/pkg/storage/drivers/memory/memory.go @@ -188,13 +188,16 @@ func (memory *memoryDriver) CreateObject(bucket, key, contentType, md5sum string } // CreateBucket - create bucket in memory -func (memory *memoryDriver) CreateBucket(bucketName string) error { +func (memory *memoryDriver) CreateBucket(bucketName, acl string) error { memory.lock.RLock() if !drivers.IsValidBucket(bucketName) { memory.lock.RUnlock() return drivers.BucketNameInvalid{Bucket: bucketName} } - + if !drivers.IsValidBucketACL(acl) { + memory.lock.RUnlock() + return drivers.InvalidACL{ACL: acl} + } if _, ok := memory.bucketMetadata[bucketName]; ok == true { memory.lock.RUnlock() return drivers.BucketExists{Bucket: bucketName} @@ -205,6 +208,7 @@ func (memory *memoryDriver) CreateBucket(bucketName string) error { newBucket.metadata = drivers.BucketMetadata{} newBucket.metadata.Name = bucketName newBucket.metadata.Created = time.Now() + newBucket.metadata.ACL = drivers.BucketACL(acl) memory.lock.Lock() defer memory.lock.Unlock() memory.bucketMetadata[bucketName] = newBucket @@ -234,7 +238,7 @@ func (memory *memoryDriver) filterDelimiterPrefix(keys []string, key, delimitedN switch true { case key == resources.Prefix: keys = appendUniq(keys, key) - // DelimitedName - requires resources.Prefix as it was trimmed off earlier in the flow + // DelimitedName - requires resources.Prefix as it was trimmed off earlier in the flow case key == resources.Prefix+delimitedName: keys = appendUniq(keys, key) case delimitedName != "": @@ -339,8 +343,6 @@ func (memory *memoryDriver) evictObject(key lru.Key, value interface{}) { k := key.(string) memory.totalSize = memory.totalSize - memory.objectMetadata[k].metadata.Size - log.Println("evicting:", k) - delete(memory.objectMetadata, k) } diff --git a/pkg/storage/drivers/mocks/Driver.go b/pkg/storage/drivers/mocks/Driver.go index 5995328fd..e94472951 100644 --- a/pkg/storage/drivers/mocks/Driver.go +++ b/pkg/storage/drivers/mocks/Driver.go @@ -27,8 +27,8 @@ func (m *Driver) ListBuckets() ([]drivers.BucketMetadata, error) { } // CreateBucket is a mock -func (m *Driver) CreateBucket(bucket string) error { - ret := m.Called(bucket) +func (m *Driver) CreateBucket(bucket, acl string) error { + ret := m.Called(bucket, acl) r0 := ret.Error(0) From 848c4ee31c8888f1934055cabbb88f19619c610d Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 22 Apr 2015 19:29:39 -0700 Subject: [PATCH 5/5] Further fixes for ACL support, currently code is disabled in all the handlers Disabled because due to lack of testing support. Once we get that in we can uncomment them back. --- pkg/api/acl.go | 33 +++++++++++--------- pkg/api/api_bucket_handlers.go | 18 +++++++++++ pkg/api/api_generic_handlers.go | 25 +--------------- pkg/api/api_object_handlers.go | 53 +++++++++++++++++++++++---------- pkg/api/api_router.go | 2 +- pkg/api/api_test.go | 2 +- pkg/api/contenttype.go | 21 ++++++------- pkg/api/errors.go | 5 ++-- 8 files changed, 91 insertions(+), 68 deletions(-) diff --git a/pkg/api/acl.go b/pkg/api/acl.go index 2fef7e679..a63b2a7bb 100644 --- a/pkg/api/acl.go +++ b/pkg/api/acl.go @@ -16,10 +16,7 @@ package api -import ( - "net/http" - "strings" -) +import "net/http" // Please read for more information - http://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl // @@ -41,16 +38,20 @@ const ( // Get acl type requested from 'x-amz-acl' header func getACLType(req *http.Request) ACLType { aclHeader := req.Header.Get("x-amz-acl") - switch { - case strings.HasPrefix(aclHeader, "private"): - return privateACLType - case strings.HasPrefix(aclHeader, "public-read"): - return publicReadACLType - case strings.HasPrefix(aclHeader, "public-read-write"): - return publicReadWriteACLType - default: - return unsupportedACLType + if aclHeader != "" { + switch { + case aclHeader == "private": + return privateACLType + case aclHeader == "public-read": + return publicReadACLType + case aclHeader == "public-read-write": + return publicReadWriteACLType + default: + return unsupportedACLType + } } + // make it default private + return privateACLType } // ACL type to human readable string @@ -68,7 +69,11 @@ func getACLTypeString(acl ACLType) string { { return "public-read-write" } + case unsupportedACLType: + { + return "" + } default: - return "" + return "private" } } diff --git a/pkg/api/api_bucket_handlers.go b/pkg/api/api_bucket_handlers.go index e17031c55..e530b5dc4 100644 --- a/pkg/api/api_bucket_handlers.go +++ b/pkg/api/api_bucket_handlers.go @@ -45,6 +45,15 @@ func (server *minioAPI) listObjectsHandler(w http.ResponseWriter, req *http.Requ vars := mux.Vars(req) bucket := vars["bucket"] + + // Enable this after tests supports them + // verify for if bucket is private or public + // bucketMetadata, err := server.driver.GetBucketMetadata(bucket) + // if err != nil || (stripAccessKey(req) == "" && bucketMetadata.ACL.IsPrivate()) { + // writeErrorResponse(w, req, AccessDenied, acceptsContentType, req.URL.Path) + // return + // } + objects, resources, err := server.driver.ListObjects(bucket, resources) switch err.(type) { case nil: // success @@ -167,6 +176,15 @@ func (server *minioAPI) headBucketHandler(w http.ResponseWriter, req *http.Reque vars := mux.Vars(req) bucket := vars["bucket"] + + // Enable this after tests supports them + // verify for if bucket is private or public + // bucketMetadata, err := server.driver.GetBucketMetadata(bucket) + // if err != nil || (stripAccessKey(req) == "" && bucketMetadata.ACL.IsPrivate()) { + // writeErrorResponse(w, req, AccessDenied, acceptsContentType, req.URL.Path) + // return + // } + _, err := server.driver.GetBucketMetadata(bucket) switch err.(type) { case nil: diff --git a/pkg/api/api_generic_handlers.go b/pkg/api/api_generic_handlers.go index f26a313e6..4202d6bc6 100644 --- a/pkg/api/api_generic_handlers.go +++ b/pkg/api/api_generic_handlers.go @@ -20,14 +20,11 @@ import ( "net/http" "strings" - "github.com/gorilla/mux" "github.com/minio-io/minio/pkg/api/config" - "github.com/minio-io/minio/pkg/storage/drivers" ) type vHandler struct { conf config.Config - driver drivers.Driver handler http.Handler } @@ -50,10 +47,9 @@ func stripAccessKey(r *http.Request) string { // Validate handler is wrapper handler used for API request validation with authorization header. // Current authorization layer supports S3's standard HMAC based signature request. -func validateHandler(conf config.Config, driver drivers.Driver, h http.Handler) http.Handler { +func validateHandler(conf config.Config, h http.Handler) http.Handler { return vHandler{ conf: conf, - driver: driver, handler: h, } } @@ -66,25 +62,6 @@ func (h vHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { writeErrorResponse(w, r, NotAcceptable, acceptsContentType, r.URL.Path) return } - // verify for if bucket is private or public - vars := mux.Vars(r) - bucket, ok := vars["bucket"] - if ok { - bucketMetadata, err := h.driver.GetBucketMetadata(bucket) - if err != nil { - writeErrorResponse(w, r, AccessDenied, acceptsContentType, r.URL.Path) - return - } - if accessKey == "" && bucketMetadata.ACL.IsPrivate() { - writeErrorResponse(w, r, AccessDenied, acceptsContentType, r.URL.Path) - return - } - if r.Method == "PUT" && bucketMetadata.ACL.IsPublicRead() { - writeErrorResponse(w, r, AccessDenied, acceptsContentType, r.URL.Path) - return - } - } - switch true { case accessKey != "": if err := h.conf.ReadConfig(); err != nil { diff --git a/pkg/api/api_object_handlers.go b/pkg/api/api_object_handlers.go index 152a645ac..492072f17 100644 --- a/pkg/api/api_object_handlers.go +++ b/pkg/api/api_object_handlers.go @@ -40,6 +40,16 @@ func (server *minioAPI) getObjectHandler(w http.ResponseWriter, req *http.Reques vars := mux.Vars(req) bucket = vars["bucket"] object = vars["object"] + + // Enable this after tests supports them + + // verify for if bucket is private or public + // bucketMetadata, err := server.driver.GetBucketMetadata(bucket) + // if err != nil || (stripAccessKey(req) == "" && bucketMetadata.ACL.IsPrivate()) { + // writeErrorResponse(w, req, AccessDenied, acceptsContentType, req.URL.Path) + // return + // } + metadata, err := server.driver.GetObjectMetadata(bucket, object, "") switch err := err.(type) { case nil: // success @@ -51,23 +61,18 @@ func (server *minioAPI) getObjectHandler(w http.ResponseWriter, req *http.Reques } switch httpRange.start == 0 && httpRange.length == 0 { case true: - { - setObjectHeaders(w, metadata) - if _, err := server.driver.GetObject(w, bucket, object); err != nil { - // unable to write headers, we've already printed data. Just close the connection. - log.Error.Println(err) - } + setObjectHeaders(w, metadata) + if _, err := server.driver.GetObject(w, bucket, object); err != nil { + // unable to write headers, we've already printed data. Just close the connection. + log.Error.Println(err) } case false: - { - metadata.Size = httpRange.length - setRangeObjectHeaders(w, metadata, httpRange) - w.WriteHeader(http.StatusPartialContent) - _, err := server.driver.GetPartialObject(w, bucket, object, httpRange.start, httpRange.length) - if err != nil { - // unable to write headers, we've already printed data. Just close the connection. - log.Error.Println(iodine.New(err, nil)) - } + metadata.Size = httpRange.length + setRangeObjectHeaders(w, metadata, httpRange) + w.WriteHeader(http.StatusPartialContent) + if _, err := server.driver.GetPartialObject(w, bucket, object, httpRange.start, httpRange.length); err != nil { + // unable to write headers, we've already printed data. Just close the connection. + log.Error.Println(iodine.New(err, nil)) } } } @@ -109,6 +114,15 @@ func (server *minioAPI) headObjectHandler(w http.ResponseWriter, req *http.Reque vars := mux.Vars(req) bucket = vars["bucket"] object = vars["object"] + + // verify for if bucket is private or public + // verify for if bucket is private or public + // bucketMetadata, err := server.driver.GetBucketMetadata(bucket) + // if err != nil || (stripAccessKey(req) == "" && bucketMetadata.ACL.IsPrivate()) { + // writeErrorResponse(w, req, AccessDenied, acceptsContentType, req.URL.Path) + // return + // } + metadata, err := server.driver.GetObjectMetadata(bucket, object, "") switch err := err.(type) { case nil: @@ -146,6 +160,15 @@ func (server *minioAPI) putObjectHandler(w http.ResponseWriter, req *http.Reques vars := mux.Vars(req) bucket = vars["bucket"] object = vars["object"] + + // verify for if bucket is private or public + // verify for if bucket is private or public + // bucketMetadata, err := server.driver.GetBucketMetadata(bucket) + // if err != nil || (stripAccessKey(req) == "" && bucketMetadata.ACL.IsPrivate()) || bucketMetadtata.ACL.IsPublicRead() { + // writeErrorResponse(w, req, AccessDenied, acceptsContentType, req.URL.Path) + // return + // } + // get Content-MD5 sent by client and verify if valid md5 := req.Header.Get("Content-MD5") if !isValidMD5(md5) { diff --git a/pkg/api/api_router.go b/pkg/api/api_router.go index d4b2d65d7..0e5813a52 100644 --- a/pkg/api/api_router.go +++ b/pkg/api/api_router.go @@ -89,5 +89,5 @@ func HTTPHandler(domain string, driver drivers.Driver) http.Handler { log.Fatal(iodine.New(err, map[string]string{"domain": domain})) } - return validateHandler(conf, api.driver, ignoreResourcesHandler(mux)) + return validateHandler(conf, ignoreResourcesHandler(mux)) } diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 474296c43..477a22209 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -115,11 +115,11 @@ func (s *MySuite) TestNonExistantObject(c *C) { } } driver := s.Driver - s.MockDriver.On("GetObjectMetadata", "bucket", "object", "").Return(drivers.ObjectMetadata{}, drivers.BucketNotFound{Bucket: "bucket"}).Once() httpHandler := api.HTTPHandler("", driver) testServer := httptest.NewServer(httpHandler) defer testServer.Close() + s.MockDriver.On("GetObjectMetadata", "bucket", "object", "").Return(drivers.ObjectMetadata{}, drivers.BucketNotFound{Bucket: "bucket"}).Once() response, err := http.Get(testServer.URL + "/bucket/object") c.Assert(err, IsNil) c.Assert(response.StatusCode, Equals, http.StatusNotFound) diff --git a/pkg/api/contenttype.go b/pkg/api/contenttype.go index 75afd7e64..aac3d3451 100644 --- a/pkg/api/contenttype.go +++ b/pkg/api/contenttype.go @@ -29,17 +29,18 @@ const ( // Get content type requested from 'Accept' header func getContentType(req *http.Request) contentType { acceptHeader := req.Header.Get("Accept") - if acceptHeader != "" { - switch { - case acceptHeader == "application/json": - return jsonContentType - case acceptHeader == "application/xml": - return xmlContentType - default: - return unknownContentType - } + switch { + case acceptHeader == "application/json": + return jsonContentType + case acceptHeader == "application/xml": + return xmlContentType + case acceptHeader == "*/*": + return xmlContentType + case acceptHeader != "": + return unknownContentType + default: + return xmlContentType } - return xmlContentType } // Content type to human readable string diff --git a/pkg/api/errors.go b/pkg/api/errors.go index d316be67e..e7908b5e7 100644 --- a/pkg/api/errors.go +++ b/pkg/api/errors.go @@ -176,9 +176,8 @@ var errorCodeResponse = map[int]Error{ HTTPStatusCode: http.StatusBadRequest, }, NotAcceptable: { - Code: "NotAcceptable", - Description: `The requested resource is only capable of generating content - not acceptable according to the Accept headers sent in the request.`, + Code: "NotAcceptable", + Description: "The requested resource is only capable of generating content not acceptable according to the Accept headers sent in the request.", HTTPStatusCode: http.StatusNotAcceptable, }, }