Revert "bucket: refactor policies and fix bugs related to enforcing policies. (#2766)"

This reverts commit ca5ca8332b.

cmd/bucket-handlers.go

@@ -31,9 +31,8 @@ import (
 // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html
 // Enforces bucket policies for a bucket for a given tatusaction.
 func enforceBucketPolicy(bucket string, action string, reqURL *url.URL) (s3Error APIErrorCode) {
-	// Fetch bucket policy, if policy is not set return access denied.
-	policy, err := readBucketPolicy(bucket, newObjectLayerFn())
-	if err != nil {
+	// Verify if bucket actually exists
+	if err := isBucketExist(bucket, newObjectLayerFn()); err != nil {
 		err = errorCause(err)
 		switch err.(type) {
 		case BucketNameInvalid:
@@ -42,16 +41,18 @@ func enforceBucketPolicy(bucket string, action string, reqURL *url.URL) (s3Error
 		case BucketNotFound:
 			// For no bucket found we return NoSuchBucket instead.
 			return ErrNoSuchBucket
-		case BucketPolicyNotFound:
-			// For no bucket policy found, return AccessDenied, since
-			// anonymous requests are not allowed without bucket policies.
-			return ErrAccessDenied
 		}
 		errorIf(err, "Unable to read bucket policy.")
 		// Return internal error for any other errors so that we can investigate.
 		return ErrInternalError
 	}
 
+	// Fetch bucket policy, if policy is not set return access denied.
+	policy := globalBucketPolicies.GetBucketPolicy(bucket)
+	if policy == nil {
+		return ErrAccessDenied
+	}
+
 	// Construct resource in 'arn:aws:s3:::examplebucket/object' format.
 	resource := AWSResourcePrefix + strings.TrimSuffix(strings.TrimPrefix(reqURL.Path, "/"), "/")
 

cmd/bucket-handlers_test.go

@@ -30,6 +30,8 @@ func TestGetBucketLocationHandler(t *testing.T) {
 }
 
 func testGetBucketLocationHandler(obj ObjectLayer, instanceType string, t TestErrHandler) {
+	initBucketPolicies(obj)
+
 	// get random bucket name.
 	bucketName := getRandomBucketName()
 	// Create bucket.
@@ -128,6 +130,8 @@ func TestHeadBucketHandler(t *testing.T) {
 }
 
 func testHeadBucketHandler(obj ObjectLayer, instanceType string, t TestErrHandler) {
+	initBucketPolicies(obj)
+
 	// get random bucket name.
 	bucketName := getRandomBucketName()
 	// Create bucket.
@@ -203,6 +207,8 @@ func TestListMultipartUploadsHandler(t *testing.T) {
 
 // testListMultipartUploadsHandler - Tests validate listing of multipart uploads.
 func testListMultipartUploadsHandler(obj ObjectLayer, instanceType string, t TestErrHandler) {
+	initBucketPolicies(obj)
+
 	// get random bucket name.
 	bucketName := getRandomBucketName()
 

cmd/bucket-policy-handlers.go

@@ -196,6 +196,9 @@ func (api objectAPIHandlers) PutBucketPolicyHandler(w http.ResponseWriter, r *ht
 		return
 	}
 
+	// Set the bucket policy in memory.
+	globalBucketPolicies.SetBucketPolicy(bucket, policy)
+
 	// Success.
 	writeSuccessNoContent(w)
 }
@@ -239,6 +242,9 @@ func (api objectAPIHandlers) DeleteBucketPolicyHandler(w http.ResponseWriter, r
 		return
 	}
 
+	// Remove bucket policy.
+	globalBucketPolicies.RemoveBucketPolicy(bucket)
+
 	// Success.
 	writeSuccessNoContent(w)
 }

cmd/bucket-policy-handlers_test.go

@@ -247,6 +247,8 @@ func TestPutBucketPolicyHandler(t *testing.T) {
 // testPutBucketPolicyHandler - Test for Bucket policy end point.
 // TODO: Add exhaustive cases with various combination of statement fields.
 func testPutBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestErrHandler) {
+	initBucketPolicies(obj)
+
 	// get random bucket name.
 	bucketName := getRandomBucketName()
 	// Create bucket.
@@ -311,6 +313,8 @@ func TestGetBucketPolicyHandler(t *testing.T) {
 // testGetBucketPolicyHandler - Test for end point which fetches the access policy json of the given bucket.
 // TODO: Add exhaustive cases with various combination of statement fields.
 func testGetBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestErrHandler) {
+	initBucketPolicies(obj)
+
 	// get random bucket name.
 	bucketName := getRandomBucketName()
 	// Create bucket.
@@ -419,6 +423,8 @@ func TestDeleteBucketPolicyHandler(t *testing.T) {
 // testDeleteBucketPolicyHandler - Test for Delete bucket policy end point.
 // TODO: Add exhaustive cases with various combination of statement fields.
 func testDeleteBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestErrHandler) {
+	initBucketPolicies(obj)
+
 	// get random bucket name.
 	bucketName := getRandomBucketName()
 	// Create bucket.

cmd/bucket-policy.go

@@ -18,10 +18,110 @@ package cmd
 
 import (
 	"bytes"
+	"errors"
 	"io"
 	"path"
+	"sync"
 )
 
+// Variable represents bucket policies in memory.
+var globalBucketPolicies *bucketPolicies
+
+// Global bucket policies list, policies are enforced on each bucket looking
+// through the policies here.
+type bucketPolicies struct {
+	rwMutex *sync.RWMutex
+
+	// Collection of 'bucket' policies.
+	bucketPolicyConfigs map[string]*bucketPolicy
+}
+
+// Fetch bucket policy for a given bucket.
+func (bp bucketPolicies) GetBucketPolicy(bucket string) *bucketPolicy {
+	bp.rwMutex.RLock()
+	defer bp.rwMutex.RUnlock()
+	return bp.bucketPolicyConfigs[bucket]
+}
+
+// Set a new bucket policy for a bucket, this operation will overwrite
+// any previous bucketpolicies for the bucket.
+func (bp *bucketPolicies) SetBucketPolicy(bucket string, policy *bucketPolicy) error {
+	bp.rwMutex.Lock()
+	defer bp.rwMutex.Unlock()
+	if policy == nil {
+		return errors.New("invalid argument")
+	}
+	bp.bucketPolicyConfigs[bucket] = policy
+	return nil
+}
+
+// Remove bucket policy for a bucket, from in-memory map.
+func (bp *bucketPolicies) RemoveBucketPolicy(bucket string) {
+	bp.rwMutex.Lock()
+	defer bp.rwMutex.Unlock()
+	delete(bp.bucketPolicyConfigs, bucket)
+}
+
+// Loads all bucket policies from persistent layer.
+func loadAllBucketPolicies(objAPI ObjectLayer) (policies map[string]*bucketPolicy, err error) {
+	// List buckets to proceed loading all notification configuration.
+	buckets, err := objAPI.ListBuckets()
+	errorIf(err, "Unable to list buckets.")
+	err = errorCause(err)
+	if err != nil {
+		return nil, err
+	}
+
+	policies = make(map[string]*bucketPolicy)
+	var pErrs []error
+	// Loads bucket policy.
+	for _, bucket := range buckets {
+		policy, pErr := readBucketPolicy(bucket.Name, objAPI)
+		if pErr != nil {
+			switch pErr.(type) {
+			case BucketPolicyNotFound:
+				continue
+			}
+			pErrs = append(pErrs, pErr)
+			// Continue to load other bucket policies if possible.
+			continue
+		}
+		policies[bucket.Name] = policy
+	}
+
+	// Look for any errors occurred while reading bucket policies.
+	for _, pErr := range pErrs {
+		if pErr != nil {
+			return policies, pErr
+		}
+	}
+
+	// Success.
+	return policies, nil
+}
+
+// Intialize all bucket policies.
+func initBucketPolicies(objAPI ObjectLayer) error {
+	if objAPI == nil {
+		return errInvalidArgument
+	}
+
+	// Read all bucket policies.
+	policies, err := loadAllBucketPolicies(objAPI)
+	if err != nil {
+		return err
+	}
+
+	// Populate global bucket collection.
+	globalBucketPolicies = &bucketPolicies{
+		rwMutex:             &sync.RWMutex{},
+		bucketPolicyConfigs: policies,
+	}
+
+	// Success.
+	return nil
+}
+
 // getOldBucketsConfigPath - get old buckets config path. (Only used for migrating old bucket policies)
 func getOldBucketsConfigPath() (string, error) {
 	configPath, err := getConfigPath()

cmd/controller-handlers.go

@@ -99,7 +99,7 @@ type HealObjectArgs struct {
 // HealObjectReply - reply by HealObject RPC.
 type HealObjectReply struct{}
 
-// HealObject - heals an object, returns nil error upon success.
+// HealObject - heal the object.
 func (c *controllerAPIHandlers) HealObjectHandler(args *HealObjectArgs, reply *GenericReply) error {
 	objAPI := c.ObjectAPI()
 	if objAPI == nil {
@@ -111,7 +111,7 @@ func (c *controllerAPIHandlers) HealObjectHandler(args *HealObjectArgs, reply *G
 	return objAPI.HealObject(args.Bucket, args.Object)
 }
 
-// HealDiskMetadataHandler - heals disks metadata, returns nil error upon success.
+// HealObject - heal the object.
 func (c *controllerAPIHandlers) HealDiskMetadataHandler(args *GenericArgs, reply *GenericReply) error {
 	objAPI := c.ObjectAPI()
 	if objAPI == nil {

cmd/routers.go

@@ -73,6 +73,10 @@ func newObjectLayer(disks, ignoredDisks []string) (ObjectLayer, error) {
 	err = initEventNotifier(objAPI)
 	fatalIf(err, "Unable to initialize event notification.")
 
+	// Initialize and load bucket policies.
+	err = initBucketPolicies(objAPI)
+	fatalIf(err, "Unable to load all bucket policies.")
+
 	// Success.
 	return objAPI, nil
 }

cmd/storage-errors.go

@@ -59,3 +59,6 @@ var errVolumeAccessDenied = errors.New("volume access denied")
 
 // errVolumeAccessDenied - cannot access file, insufficient permissions.
 var errFileAccessDenied = errors.New("file access denied")
+
+// errVolumeBusy - remote disk is not connected to yet.
+var errVolumeBusy = errors.New("volume is busy")

cmd/test-utils_test.go

@@ -1298,7 +1298,6 @@ func initTestAPIEndPoints(objLayer ObjectLayer, apiFunctions []string) http.Hand
 	// initialize a new mux router.
 	// goriilla/mux is the library used to register all the routes and handle them.
 	muxRouter := router.NewRouter()
-
 	// All object storage operations are registered as HTTP handlers on `objectAPIHandlers`.
 	// When the handlers get a HTTP request they use the underlyting ObjectLayer to perform operations.
 	objLayerMutex.Lock()

cmd/web-handlers.go

@@ -128,7 +128,7 @@ func (web *webAPIHandlers) StorageInfo(r *http.Request, args *GenericArgs, reply
 	reply.UIVersion = miniobrowser.UIVersion
 	objectAPI := web.ObjectAPI()
 	if objectAPI == nil {
-		return &json2.Error{Message: "Server not initialized, please try again., please try again."}
+		return &json2.Error{Message: "Server not initialized"}
 	}
 	reply.StorageInfo = objectAPI.StorageInfo()
 	return nil
@@ -147,7 +147,7 @@ func (web *webAPIHandlers) MakeBucket(r *http.Request, args *MakeBucketArgs, rep
 	reply.UIVersion = miniobrowser.UIVersion
 	objectAPI := web.ObjectAPI()
 	if objectAPI == nil {
-		return &json2.Error{Message: "Server not initialized, please try again."}
+		return &json2.Error{Message: "Server not initialized"}
 	}
 	if err := objectAPI.MakeBucket(args.BucketName); err != nil {
 		return &json2.Error{Message: err.Error()}
@@ -176,7 +176,7 @@ func (web *webAPIHandlers) ListBuckets(r *http.Request, args *WebGenericArgs, re
 	}
 	objectAPI := web.ObjectAPI()
 	if objectAPI == nil {
-		return &json2.Error{Message: "Server not initialized, please try again."}
+		return &json2.Error{Message: "Server not initialized"}
 	}
 	buckets, err := objectAPI.ListBuckets()
 	if err != nil {
@@ -228,7 +228,7 @@ func (web *webAPIHandlers) ListObjects(r *http.Request, args *ListObjectsArgs, r
 	for {
 		objectAPI := web.ObjectAPI()
 		if objectAPI == nil {
-			return &json2.Error{Message: "Server not initialized, please try again."}
+			return &json2.Error{Message: "Server not initialized"}
 		}
 		lo, err := objectAPI.ListObjects(args.BucketName, args.Prefix, marker, "/", 1000)
 		if err != nil {
@@ -270,7 +270,7 @@ func (web *webAPIHandlers) RemoveObject(r *http.Request, args *RemoveObjectArgs,
 	reply.UIVersion = miniobrowser.UIVersion
 	objectAPI := web.ObjectAPI()
 	if objectAPI == nil {
-		return &json2.Error{Message: "Server not initialized, please try again."}
+		return &json2.Error{Message: "Server not initialized"}
 	}
 	if err := objectAPI.DeleteObject(args.BucketName, args.ObjectName); err != nil {
 		return &json2.Error{Message: err.Error()}
@@ -413,7 +413,7 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) {
 
 	objectAPI := web.ObjectAPI()
 	if objectAPI == nil {
-		writeWebErrorResponse(w, errors.New("Server not initialized, please try again."))
+		writeWebErrorResponse(w, errors.New("Server not initialized"))
 		return
 	}
 	if _, err := objectAPI.PutObject(bucket, object, -1, r.Body, metadata); err != nil {
@@ -469,7 +469,7 @@ func (web *webAPIHandlers) Download(w http.ResponseWriter, r *http.Request) {
 
 	objectAPI := web.ObjectAPI()
 	if objectAPI == nil {
-		writeWebErrorResponse(w, errors.New("Server not initialized, please try again."))
+		writeWebErrorResponse(w, errors.New("Server not initialized"))
 		return
 	}
 	objInfo, err := objectAPI.GetObjectInfo(bucket, object)
@@ -568,7 +568,7 @@ func (web *webAPIHandlers) GetBucketPolicy(r *http.Request, args *GetBucketPolic
 
 	objectAPI := web.ObjectAPI()
 	if objectAPI == nil {
-		return &json2.Error{Message: "Server not initialized, please try again."}
+		return &json2.Error{Message: "Server not initialized"}
 	}
 	policyInfo, err := readBucketAccessPolicy(objectAPI, args.BucketName)
 	if err != nil {
@@ -638,7 +638,7 @@ func (web *webAPIHandlers) SetBucketPolicy(r *http.Request, args *SetBucketPolic
 	}
 	objectAPI := web.ObjectAPI()
 	if objectAPI == nil {
-		return &json2.Error{Message: "Server not initialized, please try again."}
+		return &json2.Error{Message: "Server not initialized"}
 	}
 
 	bucketP := policy.BucketPolicy(args.Policy)