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

@@ -16,10 +16,7 @@
 
 package api
 
-import (
+import "net/http"
-	"net/http"
-	"strings"
-)
 
 // 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")
+	if aclHeader != "" {
 		switch {
-	case strings.HasPrefix(aclHeader, "private"):
+		case aclHeader == "private":
 			return privateACLType
-	case strings.HasPrefix(aclHeader, "public-read"):
+		case aclHeader == "public-read":
 			return publicReadACLType
-	case strings.HasPrefix(aclHeader, "public-read-write"):
+		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"
 		}
-	default:
+	case unsupportedACLType:
+		{
 			return ""
 		}
+	default:
+		return "private"
+	}
 }

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:

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 {

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,26 +61,21 @@ 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)
 				}
-				}
 			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 := server.driver.GetPartialObject(w, bucket, object, httpRange.start, httpRange.length); err != nil {
-					if err != nil {
 					// unable to write headers, we've already printed data. Just close the connection.
 					log.Error.Println(iodine.New(err, nil))
 				}
 			}
 		}
-		}
 	case drivers.ObjectNotFound:
 		{
 			writeErrorResponse(w, req, NoSuchKey, acceptsContentType, req.URL.Path)
@@ -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) {

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))
 }

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)

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:
+	case acceptHeader == "*/*":
-			return unknownContentType
-		}
-	}
 		return xmlContentType
+	case acceptHeader != "":
+		return unknownContentType
+	default:
+		return xmlContentType
+	}
 }
 
 // Content type to human readable string

pkg/api/errors.go

@@ -177,8 +177,7 @@ var errorCodeResponse = map[int]Error{
 	},
 	NotAcceptable: {
 		Code:           "NotAcceptable",
-		Description: `The requested resource is only capable of generating content
+		Description:    "The requested resource is only capable of generating content not acceptable according to the Accept headers sent in the request.",
-                              not acceptable according to the Accept headers sent in the request.`,
 		HTTPStatusCode: http.StatusNotAcceptable,
 	},
 }