From 848c4ee31c8888f1934055cabbb88f19619c610d Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 22 Apr 2015 19:29:39 -0700 Subject: [PATCH] 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, }, }