ACL Handlers

To make controller classes clean, this patch introduces new
mechanism called ACL Handlers. It is really useful for customizing
acl checking algorithms for each controller.

BaseAclHandler wraps basic Acl handling as same as current one.
(i.e. it will check acl from ACL_MAP by using HEAD)

In addition, we can make some extended custom classes with the same
name of the controllers. (e.g. BucketAclHandler is for BucketController)
They consist of method(s) for actual S3 method on controllers as follows.

e.g.:
class BucketAclHandler(BaseAclHandler):
   def PUT:
       << put acl handling algorithms here for PUT bucket >>

Change-Id: I155cd6387c81c03a2092ecd933f4769e5148c591

swift3/acl_handlers.py

@@ -0,0 +1,384 @@
+# Copyright (c) 2014 OpenStack Foundation.
+#
+# 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.
+
+from swift3.subresource import ACL, Owner
+from swift3.response import NoSuchKey, \
+    MissingSecurityHeader, MalformedACLError, UnexpectedContent
+from swift3.etree import fromstring, XMLSyntaxError, DocumentInvalid
+from swift3.utils import LOGGER, MULTIUPLOAD_SUFFIX
+
+from swift.common.utils import split_path
+
+"""
+Acl Handlers:
+
+Why do we need this:
+To make controller classes clean, we need these handlers.
+It is really useful for customizing acl checking algorithms for
+each controller.
+
+Basic Information:
+BaseAclHandler wraps basic Acl handling.
+(i.e. it will check acl from ACL_MAP by using HEAD)
+
+How to extend:
+Make a handler with the name of the controller.
+(e.g. BucketAclHandler is for BucketController)
+It consists of method(s) for actual S3 method on controllers as follows.
+
+e.g.:
+class BucketAclHandler(BaseAclHandler):
+   def PUT:
+       << put acl handling algorithms here for PUT bucket >>
+
+NOTE:
+If the method DON'T need to recall _get_response in outside of
+acl checking, the method have to return the response it needs at
+the end of method.
+"""
+
+
+def get_acl(headers, body, bucket_owner, object_owner=None):
+    """
+    Get ACL instance from S3 (e.g. x-amz-grant) headers or S3 acl xml body.
+    """
+    acl = ACL.from_headers(headers, bucket_owner, object_owner,
+                           as_private=False)
+
+    if acl is None:
+        # Get acl from request body if possible.
+        if not body:
+            msg = 'Your request was missing a required header'
+            raise MissingSecurityHeader(msg, missing_header_name='x-amz-acl')
+        try:
+            elem = fromstring(body, ACL.root_tag)
+            acl = ACL.from_elem(elem)
+        except(XMLSyntaxError, DocumentInvalid):
+            raise MalformedACLError()
+        except Exception as e:
+            LOGGER.error(e)
+            raise
+    else:
+        if body:
+            # Specifying grant with both header and xml is not allowed.
+            raise UnexpectedContent
+
+    return acl
+
+
+def get_acl_handler(controller_name):
+    for base_klass in [BaseAclHandler, MultiUploadAclHandler]:
+        # pylint: disable-msg=E1101
+        for handler in base_klass.__subclasses__():
+            handler_suffix_len = len('AclHandler') \
+                if not handler.__name__ == 'S3AclHandler' else len('Hanlder')
+            if handler.__name__[:-handler_suffix_len] == controller_name:
+                return handler
+    return BaseAclHandler
+
+
+class BaseAclHandler(object):
+    """
+    BaseAclHandler: Handling ACL for basic requests mapped on ACL_MAP
+    """
+    def __init__(self, req, container, obj):
+        self.req = req
+        self.container = self.req.container_name if container is None \
+            else container
+        self.obj = self.req.object_name if obj is None else obj
+        self.method = req.environ['REQUEST_METHOD']
+        self.user_id = self.req.user_id
+
+    def _check_copy_source(self, app):
+        if 'X-Amz-Copy-Source' in self.req.headers:
+            src_path = self.req.headers['X-Amz-Copy-Source']
+            src_path = src_path if src_path.startswith('/') else \
+                ('/' + src_path)
+            src_bucket, src_obj = split_path(src_path, 0, 2, True)
+            self._handle_acl(app, 'HEAD', src_bucket, src_obj, 'READ')
+
+    def handle_acl(self, app, method):
+        method = method or self.method
+        if hasattr(self, method):
+            return getattr(self, method)(app)
+        else:
+            return self._handle_acl(app, method)
+
+    def _handle_acl(self, app, sw_method, container=None, obj=None,
+                    permission=None):
+        """
+        General acl handling method.
+        This method expects to call Request._get_response() in outside of
+        this method so that this method returns resonse only when sw_method
+        is HEAD.
+        """
+
+        container = self.container if container is None else container
+        obj = self.obj if obj is None else obj
+        sw_method = sw_method or self.req.environ['REQUEST_METHOD']
+        resource = 'object' if obj else 'container'
+
+        if not container:
+            return
+
+        if not permission and (self.method, sw_method, resource) in ACL_MAP:
+            acl_check = ACL_MAP[(self.method, sw_method, resource)]
+            resource = acl_check.get('Resource') or resource
+            permission = acl_check['Permission']
+
+        if not permission:
+            raise Exception('No permission to be checked exists')
+
+        if resource == 'object':
+            resp = self.req.get_acl_response(app, 'HEAD',
+                                             container, obj)
+            acl = resp.object_acl
+        elif resource == 'container':
+            resp = self.req.get_acl_response(app, 'HEAD',
+                                             container, '')
+            acl = resp.bucket_acl
+
+        acl.check_permission(self.user_id, permission)
+
+        if sw_method == 'HEAD':
+            return resp
+
+
+class BucketAclHandler(BaseAclHandler):
+    """
+    BucketAclHandler: Handler for BucketController
+    """
+    def PUT(self, app):
+        req_acl = ACL.from_headers(self.req.headers,
+                                   Owner(self.user_id, self.user_id))
+
+        # To avoid overwriting the existing bucket's ACL, we send PUT
+        # request first before setting the ACL to make sure that the target
+        # container does not exist.
+        self.req.get_acl_response(app, 'PUT')
+
+        # update metadata
+        self.req.bucket_acl = req_acl
+
+        # FIXME If this request is failed, there is a possibility that the
+        # bucket which has no ACL is left.
+        return self.req.get_acl_response(app, 'POST')
+
+
+class ObjectAclHandler(BaseAclHandler):
+    """
+    ObjectAclHandler: Handler for ObjectController
+    """
+    def PUT(self, app):
+        self._check_copy_source(app)
+
+        b_resp = self._handle_acl(app, 'HEAD', obj='')
+        # To avoid overwriting the existing object by unauthorized user,
+        # we send HEAD request first before writing the object to make
+        # sure that the target object does not exist or the user that sent
+        # the PUT request have write permission.
+        try:
+            self._handle_acl(app, 'HEAD')
+        except NoSuchKey:
+            pass
+        req_acl = ACL.from_headers(self.req.headers,
+                                   b_resp.bucket_acl.owner,
+                                   Owner(self.user_id, self.user_id))
+        self.req.object_acl = req_acl
+
+
+class S3AclHandler(BaseAclHandler):
+    """
+    S3AclHandler: Handler for S3AclController
+    """
+    def GET(self, app):
+        self._handle_acl(app, 'HEAD', permission='READ_ACP')
+
+    def PUT(self, app):
+        if self.req.is_object_request:
+            b_resp = self.req.get_acl_response(app, 'HEAD', obj='')
+            o_resp = self._handle_acl(app, 'HEAD', permission='WRITE_ACP')
+            req_acl = get_acl(self.req.headers,
+                              self.req.xml(ACL.max_xml_length),
+                              b_resp.bucket_acl.owner,
+                              o_resp.object_acl.owner)
+
+            # Don't change the owner of the resource by PUT acl request.
+            o_resp.object_acl.check_owner(req_acl.owner.id)
+
+            for g in req_acl.grants:
+                LOGGER.debug('Grant %s %s permission on the object /%s/%s' %
+                             (g.grantee, g.permission, self.req.container_name,
+                              self.req.object_name))
+            self.req.object_acl = req_acl
+        else:
+            self._handle_acl(app, self.method)
+
+    def POST(self, app):
+        if self.req.is_bucket_request:
+            resp = self._handle_acl(app, 'HEAD', permission='WRITE_ACP')
+
+            req_acl = get_acl(self.req.headers,
+                              self.req.xml(ACL.max_xml_length),
+                              resp.bucket_acl.owner)
+
+            # Don't change the owner of the resource by PUT acl request.
+            resp.bucket_acl.check_owner(req_acl.owner.id)
+
+            for g in req_acl.grants:
+                LOGGER.debug('Grant %s %s permission on the bucket /%s' %
+                             (g.grantee, g.permission,
+                              self.req.container_name))
+            self.req.bucket_acl = req_acl
+        else:
+            self._handle_acl(app, self.method)
+
+
+class MultiObjectDeleteAclHandler(BaseAclHandler):
+    """
+    MultiObjectDeleteAclHandler: Handler for MultiObjectDeleteController
+    """
+    def DELETE(self, app):
+        # Only bucket write acl is required
+        pass
+
+
+class MultiUploadAclHandler(BaseAclHandler):
+    """
+    MultiUpload stuff requires acl checking just once for BASE container
+    so that MultiUploadAclHandler extends BaseAclHandler to check acl only
+    when the verb defined. We should define tThe verb as the first step to
+    request to backend Swift at incoming request.
+
+    Basic Rules:
+    - BASE container name is always w/o 'MULTIUPLOAD_SUFFIX'
+    - Any check timing is ok but we should check it as soon as possible.
+
+     Controller | Verb   | CheckResource | Permission
+    --------------------------------------------------
+     Part       | PUT    | Container     | WRITE
+     Uploads    | GET    | Container     | READ
+     Uploads    | POST   | Container     | WRITE
+     Upload     | GET    | Container     | READ
+     Upload     | DELETE | Container     | WRITE
+     Upload     | POST   | Container     | WRITE
+     -------------------------------------------------
+
+    """
+    def __init__(self, req, container, obj):
+        super(MultiUploadAclHandler, self).__init__(req, container, obj)
+        self.container = self.container[:-len(MULTIUPLOAD_SUFFIX)]
+
+    def handle_acl(self, app, method):
+        method = method or self.method
+        # MultiUpload stuffs don't need acl check basically.
+        if hasattr(self, method):
+            return getattr(self, method)(app)
+        else:
+            pass
+
+    def HEAD(self, app):
+        # For _check_upload_info
+        self._handle_acl(app, 'HEAD', self.container, '')
+
+
+class PartAclHandler(MultiUploadAclHandler):
+    """
+    PartAclHandler: Handler for PartController
+    """
+    def PUT(self, app):
+        # Upload Part
+        self._check_copy_source(app)
+
+
+class UploadsAclHandler(MultiUploadAclHandler):
+    """
+    UploadsAclHandler: Handler for UploadsController
+    """
+    def GET(self, app):
+        # List Multipart Upload
+        self._handle_acl(app, 'GET', self.container, '')
+
+    def PUT(self, app):
+        if not self.obj:
+            # Initiate Multipart Uploads (put +segment container)
+            self._handle_acl(app, 'PUT', self.container)
+        # No check needed at Initiate Multipart Uploads (put upload id object)
+
+
+class UploadAclHandler(MultiUploadAclHandler):
+    """
+    UploadAclHandler: Handler for UploadController
+    """
+    def HEAD(self, app):
+        # FIXME: GET HEAD case conflicts with GET service
+        method = 'GET' if self.method == 'GET' else 'HEAD'
+        self._handle_acl(app, method, self.container, '')
+
+
+"""
+ACL_MAP =
+    {
+        ('<s3_method>', '<swift_method>', '<swift_resource>'):
+        {'Resource': '<check_resource>',
+         'Permission': '<check_permission>'},
+        ...
+    }
+
+s3_method: Method of S3 Request from user to swift3
+swift_method: Method of Swift Request from swift3 to swift
+swift_resource: Resource of Swift Request from swift3 to swift
+check_resource: <container/object>
+check_permission: <OWNER/READ/WRITE/READ_ACP/WRITE_ACP>
+"""
+ACL_MAP = {
+    # HEAD Bucket
+    ('HEAD', 'HEAD', 'container'):
+    {'Permission': 'READ'},
+    # GET Service
+    ('GET', 'HEAD', 'container'):
+    {'Permission': 'OWNER'},
+    # GET Bucket, List Parts, List Multipart Upload
+    ('GET', 'GET', 'container'):
+    {'Permission': 'READ'},
+    # PUT Object, PUT Object Copy
+    ('PUT', 'HEAD', 'container'):
+    {'Permission': 'WRITE'},
+    # DELETE Bucket
+    ('DELETE', 'DELETE', 'container'):
+    {'Permission': 'OWNER'},
+    # HEAD Object
+    ('HEAD', 'HEAD', 'object'):
+    {'Permission': 'READ'},
+    # GET Object
+    ('GET', 'GET', 'object'):
+    {'Permission': 'READ'},
+    # PUT Object, PUT Object Copy
+    ('PUT', 'HEAD', 'object'):
+    {'Permission': 'WRITE'},
+    # Initiate Multipart Upload
+    ('POST', 'PUT', 'container'):
+    {'Permission': 'WRITE'},
+    # Abort Multipart Upload
+    ('DELETE', 'HEAD', 'container'):
+    {'Permission': 'WRITE'},
+    # Delete Object
+    ('DELETE', 'DELETE', 'object'):
+    {'Resource': 'container',
+     'Permission': 'WRITE'},
+    # Complete Multipart Upload, DELETE Multiple Objects
+    ('POST', 'HEAD', 'container'):
+    {'Permission': 'WRITE'},
+}

swift3/controllers/__init__.py

@@ -19,7 +19,7 @@ from swift3.controllers.bucket import BucketController
 from swift3.controllers.obj import ObjectController
 
 from swift3.controllers.acl import AclController
-from swift3.controllers.s3_acl import AclController as S3AclController
+from swift3.controllers.s3_acl import S3AclController
 from swift3.controllers.multi_delete import MultiObjectDeleteController
 from swift3.controllers.multi_upload import UploadController, \
     PartController, UploadsController

swift3/controllers/bucket.py

@@ -24,7 +24,6 @@ from swift3.etree import Element, SubElement, tostring, fromstring, \
 from swift3.response import HTTPOk, S3NotImplemented, InvalidArgument, \
     MalformedXML, InvalidLocationConstraint
 from swift3.cfg import CONF
-from swift3.subresource import ACL, Owner
 from swift3.utils import LOGGER
 
 MAX_PUT_BUCKET_BODY_SIZE = 10240
@@ -133,25 +132,10 @@ class BucketController(Controller):
                 # Swift3 cannot support multiple reagions now.
                 raise InvalidLocationConstraint()
 
-        if CONF.s3_acl:
-            req_acl = ACL.from_headers(req.headers,
-                                       Owner(req.user_id, req.user_id))
+        if 'HTTP_X_AMZ_ACL' in req.environ:
+            handle_acl_header(req)
 
-            # To avoid overwriting the existing bucket's ACL, we send PUT
-            # request first before setting the ACL to make sure that the target
-            # container does not exist.
-            resp = req.get_response(self.app)
-
-            # update metadata
-            req.bucket_acl = req_acl
-            # FIXME If this request is failed, there is a possibility that the
-            # bucket which has no ACL is left.
-            req.get_response(self.app, 'POST')
-        else:
-            if 'HTTP_X_AMZ_ACL' in req.environ:
-                handle_acl_header(req)
-
-            resp = req.get_response(self.app)
+        resp = req.get_response(self.app)
 
         resp.status = HTTP_OK
         resp.location = '/' + req.container_name

swift3/controllers/multi_delete.py

@@ -45,9 +45,8 @@ class MultiObjectDeleteController(Controller):
 
                 yield key, version
 
-        # check bucket permission
-        if CONF.s3_acl:
-            req.get_response(self.app, 'HEAD')
+        # check bucket existence
+        req.get_response(self.app, 'HEAD')
 
         try:
             xml = req.xml(MAX_MULTI_DELETE_BODY_SIZE, check_md5=True)

swift3/controllers/multi_upload.py

@@ -69,8 +69,7 @@ def _check_upload_info(req, app, upload_id):
     obj = '%s/%s' % (req.object_name, upload_id)
 
     try:
-        req.get_response(app, 'HEAD', container=container, obj=obj,
-                         skip_check=True)
+        req.get_response(app, 'HEAD', container=container, obj=obj)
     except NoSuchKey:
         raise NoSuchUpload(upload_id=upload_id)
 
@@ -104,8 +103,6 @@ class PartController(Controller):
         upload_id = req.params['uploadId']
         _check_upload_info(req, self.app, upload_id)
 
-        req.check_copy_source(self.app)
-
         req.container_name += MULTIUPLOAD_SUFFIX
         req.object_name = '%s/%s/%d' % (req.object_name, upload_id,
                                         part_number)
@@ -337,8 +334,7 @@ class UploadController(Controller):
         objects = loads(resp.body)
         for o in objects:
             container = req.container_name + MULTIUPLOAD_SUFFIX
-            req.get_response(self.app, container=container, obj=o['name'],
-                             skip_check=True)
+            req.get_response(self.app, container=container, obj=o['name'])
 
         return HTTPNoContent()
 

swift3/controllers/obj.py

@@ -16,10 +16,8 @@
 from swift.common.http import HTTP_OK
 
 from swift3.controllers.base import Controller
-from swift3.response import AccessDenied, HTTPOk, NoSuchKey
+from swift3.response import AccessDenied, HTTPOk
 from swift3.etree import Element, SubElement, tostring
-from swift3.subresource import ACL, Owner
-from swift3.cfg import CONF
 
 
 class ObjectController(Controller):
@@ -56,24 +54,6 @@ class ObjectController(Controller):
         """
         Handle PUT Object and PUT Object (Copy) request
         """
-        if CONF.s3_acl:
-            req.check_copy_source(self.app)
-
-            b_resp = req.get_response(self.app, 'HEAD', obj='')
-            # To avoid overwriting the existing object by unauthorized user,
-            # we send HEAD request first before writing the object to make
-            # sure that the target object does not exist or the user that sent
-            # the PUT request have write permission.
-            try:
-                req.get_response(self.app, 'HEAD')
-            except NoSuchKey:
-                pass
-            req_acl = ACL.from_headers(req.headers,
-                                       b_resp.bucket_acl.owner,
-                                       Owner(req.user_id, req.user_id))
-
-            req.object_acl = req_acl
-
         resp = req.get_response(self.app)
 
         if 'X-Amz-Copy-Source' in req.headers:
@@ -83,7 +63,6 @@ class ObjectController(Controller):
             return HTTPOk(body=body, headers=resp.headers)
 
         resp.status = HTTP_OK
-
         return resp
 
     def POST(self, req):

swift3/controllers/s3_acl.py

@@ -16,42 +16,11 @@
 from urllib import quote
 
 from swift3.controllers.base import Controller
-from swift3.response import HTTPOk, MissingSecurityHeader, \
-    UnexpectedContent, MalformedACLError
-from swift3.etree import fromstring, tostring, XMLSyntaxError, DocumentInvalid
-from swift3.subresource import ACL
-from swift3.utils import LOGGER
+from swift3.response import HTTPOk
+from swift3.etree import tostring
 
 
-def get_acl(headers, body, bucket_owner, object_owner=None):
-    """
-    Get ACL instance from S3 (e.g. x-amz-grant) headers or S3 acl xml body.
-    """
-    acl = ACL.from_headers(headers, bucket_owner, object_owner,
-                           as_private=False)
-
-    if acl is None:
-        # Get acl from request body if possible.
-        if not body:
-            msg = 'Your request was missing a required header'
-            raise MissingSecurityHeader(msg, missing_header_name='x-amz-acl')
-        try:
-            elem = fromstring(body, ACL.root_tag)
-            acl = ACL.from_elem(elem)
-        except(XMLSyntaxError, DocumentInvalid):
-            raise MalformedACLError()
-        except Exception as e:
-            LOGGER.error(e)
-            raise
-    else:
-        if body:
-            # Specifying grant with both header and xml is not allowed.
-            raise UnexpectedContent
-
-    return acl
-
-
-class AclController(Controller):
+class S3AclController(Controller):
     """
     Handles the following APIs:
 
@@ -66,7 +35,7 @@ class AclController(Controller):
         """
         Handles GET Bucket acl and GET Object acl.
         """
-        resp = req.get_response(self.app, 'HEAD', permission='READ_ACP')
+        resp = req.get_response(self.app)
         acl = getattr(resp, '%s_acl' %
                       ('object' if req.is_object_request else 'bucket'))
 
@@ -80,21 +49,6 @@ class AclController(Controller):
         Handles PUT Bucket acl and PUT Object acl.
         """
         if req.is_object_request:
-            b_resp = req.get_response(self.app, 'HEAD', obj='',
-                                      skip_check=True)
-            o_resp = req.get_response(self.app, 'HEAD', permission='WRITE_ACP')
-            req_acl = get_acl(req.headers, req.xml(ACL.max_xml_length),
-                              b_resp.bucket_acl.owner,
-                              o_resp.object_acl.owner)
-
-            # Don't change the owner of the resource by PUT acl request.
-            o_resp.object_acl.check_owner(req_acl.owner.id)
-
-            for g in req_acl.grants:
-                LOGGER.debug('Grant %s %s permission on the object /%s/%s' %
-                             (g.grantee, g.permission, req.container_name,
-                              req.object_name))
-            req.object_acl = req_acl
             headers = {}
             src_path = '/%s/%s' % (req.container_name, req.object_name)
 
@@ -103,22 +57,8 @@ class AclController(Controller):
             # So headers['X-Copy-From'] for copy request is added here.
             headers['X-Copy-From'] = quote(src_path)
             headers['Content-Length'] = 0
-            req.get_response(self.app, 'PUT', headers=headers,
-                             skip_check=True)
+            req.get_response(self.app, 'PUT', headers=headers)
         else:
-            resp = req.get_response(self.app, 'HEAD', permission='WRITE_ACP')
-
-            req_acl = get_acl(req.headers, req.xml(ACL.max_xml_length),
-                              resp.bucket_acl.owner)
-
-            # Don't change the owner of the resource by PUT acl request.
-            resp.bucket_acl.check_owner(req_acl.owner.id)
-
-            for g in req_acl.grants:
-                LOGGER.debug('Grant %s %s permission on the bucket /%s' %
-                             (g.grantee, g.permission, req.container_name))
-
-            req.bucket_acl = req_acl
-            req.get_response(self.app, 'POST', skip_check=True)
+            req.get_response(self.app, 'POST')
 
         return HTTPOk()

swift3/middleware.py

@@ -57,7 +57,7 @@ from paste.deploy import loadwsgi
 from swift.common.wsgi import PipelineWrapper, loadcontext
 
 from swift3.exception import NotS3Request
-from swift3.request import Request, S3ACLRequest
+from swift3.request import Request, S3AclRequest
 from swift3.response import ErrorResponse, InternalError, MethodNotAllowed, \
     ResponseBase
 from swift3.cfg import CONF
@@ -76,7 +76,7 @@ class Swift3Middleware(object):
     def __call__(self, env, start_response):
         try:
             if CONF.s3_acl:
-                req = S3ACLRequest(env, self.app, self.slo_enabled)
+                req = S3AclRequest(env, self.app, self.slo_enabled)
             else:
                 req = Request(env, self.slo_enabled)
             resp = self.handle_request(req)

swift3/request.py

@@ -42,11 +42,11 @@ from swift3.response import AccessDenied, InvalidArgument, InvalidDigest, \
     MissingContentLength, InvalidStorageClass, S3NotImplemented, InvalidURI, \
     MalformedXML, InvalidRequest, InvalidBucketName
 from swift3.exception import NotS3Request, BadSwiftRequest
-from swift3.utils import utf8encode, LOGGER, check_path_header, \
-    MULTIUPLOAD_SUFFIX
+from swift3.utils import utf8encode, LOGGER, check_path_header
 from swift3.cfg import CONF
 from swift3.subresource import decode_acl, encode_acl
 from swift3.utils import sysmeta_header, validate_bucket_name
+from swift3.acl_handlers import get_acl_handler
 
 # List of sub-resources that must be maintained as part of the HMAC
 # signature string.
@@ -333,15 +333,9 @@ class Request(swob.Request):
 
         return buf + path
 
-    def check_copy_source(self, app):
-        if 'X-Amz-Copy-Source' in self.headers:
-            src_path = self.headers['X-Amz-Copy-Source']
-            src_path = src_path if src_path.startswith('/') else \
-                ('/' + src_path)
-            src_bucket, src_obj = split_path(src_path, 0, 2, True)
-
-            self.get_response(app, 'HEAD', src_bucket, src_obj,
-                              permission='READ')
+    @property
+    def controller_name(self):
+        return self.controller.__name__[:-len('Controller')]
 
     @property
     def controller(self):
@@ -571,6 +565,14 @@ class Request(swob.Request):
         Calls the application with this request's environment.  Returns a
         Response object that wraps up the application's result.
         """
+
+        method = method or self.environ['REQUEST_METHOD']
+
+        if container is None:
+            container = self.container_name
+        if obj is None:
+            obj = self.object_name
+
         sw_req = self.to_swift_req(method, container, obj, headers=headers,
                                    body=body, query=query)
 
@@ -615,37 +617,30 @@ class Request(swob.Request):
         raise InternalError('unexpected status code %d' % status)
 
     def get_response(self, app, method=None, container=None, obj=None,
-                     headers=None, body=None, query=None, permission=None,
-                     skip_check=False):
+                     headers=None, body=None, query=None):
         """
-        Calls the application with this request's environment.  Returns a
-        Response object that wraps up the application's result.
+        get_response is an entry point to be extended for chiled classes.
+        If additional tasks needed at that time of getting swift response,
+        we can override this method. swift3.request.Request need to just call
+        _get_response to get pure swift response.
         """
-
-        method = method or self.environ['REQUEST_METHOD']
-
-        if container is None:
-            container = self.container_name
-        if obj is None:
-            obj = self.object_name
-
         return self._get_response(app, method, container, obj,
                                   headers, body, query)
 
 
-class S3ACLRequest(Request):
+class S3AclRequest(Request):
     """
-    S3ACL request object.
+    S3Acl request object.
     """
     def __init__(self, env, app, slo_enabled=True):
-        super(S3ACLRequest, self).__init__(env, slo_enabled)
+        super(S3AclRequest, self).__init__(env, slo_enabled)
         self.authenticate(app)
 
     @property
     def controller(self):
         if 'acl' in self.params and not self.is_service_request:
             return S3AclController
-        return super(S3ACLRequest, self).controller
+        return super(S3AclRequest, self).controller
 
     def authenticate(self, app):
         """
@@ -679,7 +674,7 @@ class S3ACLRequest(Request):
 
     def to_swift_req(self, method, container, obj, query=None,
                      body=None, headers=None):
-        sw_req = super(S3ACLRequest, self).to_swift_req(
+        sw_req = super(S3AclRequest, self).to_swift_req(
             method, container, obj, query, body, headers)
         if self.account:
             sw_req.environ['swift_owner'] = True  # needed to set ACL
@@ -687,122 +682,32 @@ class S3ACLRequest(Request):
             sw_req.environ['swift.authorize'] = lambda req: None
         return sw_req
 
-    def _get_response_acl(self, app, method, container, obj,
-                          headers=None, body=None, query=None):
+    def get_acl_response(self, app, method=None, container=None, obj=None,
+                         headers=None, body=None, query=None):
+        """
+        Wrapper method of _get_response to add s3 acl information
+        from response sysmeta headers.
+        """
+
         resp = self._get_response(
             app, method, container, obj, headers, body, query)
+
         resp.bucket_acl = decode_acl('container', resp.sysmeta_headers)
         resp.object_acl = decode_acl('object', resp.sysmeta_headers)
         return resp
 
     def get_response(self, app, method=None, container=None, obj=None,
-                     headers=None, body=None, query=None, permission=None,
-                     skip_check=False):
+                     headers=None, body=None, query=None):
+        """
+        Wrap up get_response call to hook with acl handling method.
+        """
+        acl_handler = get_acl_handler(self.controller_name)(
+            self, container, obj)
+        resp = acl_handler.handle_acl(app, method)
 
-        if container is None:
-            container = self.container_name
-        if obj is None:
-            obj = self.object_name
-
-        # Skip ACL check for all requests of Account
-        if not container:
-            skip_check = True
-
-        sw_method = method or self.environ['REQUEST_METHOD']
-        resource = 'object' if obj else 'container'
-        s3_method = self.environ['REQUEST_METHOD']
-
-        if not skip_check:
-            if not permission and (s3_method, sw_method, resource) in ACL_MAP:
-                acl_check = ACL_MAP[(s3_method, sw_method, resource)]
-                resource = acl_check.get('Resource') or resource
-                permission = acl_check['Permission']
-
-            if permission:
-                org_container = container[:-len(MULTIUPLOAD_SUFFIX)] \
-                    if container.endswith(MULTIUPLOAD_SUFFIX) else container
-                match_resource = True
-                if resource == 'object':
-                    resp = self._get_response_acl(app, 'HEAD',
-                                                  org_container, obj)
-                    acl = resp.object_acl
-                elif resource == 'container':
-                    resp = self._get_response_acl(app, 'HEAD',
-                                                  org_container, None)
-                    acl = resp.bucket_acl
-                    if obj:
-                        match_resource = False
-
-                acl.check_permission(self.user_id, permission)
-
-                if sw_method == 'HEAD' and match_resource:
-                    # If the request to swift is HEAD and the resource is
-                    # consistent with the confirmation subject of ACL, not
-                    # request again. This is because that contains the
-                    # information required in the HEAD response at the time of
-                    # ACL acquisition.
-                    return resp
-        return self._get_response_acl(app, sw_method, container, obj,
-                                      headers, body, query)
-
-"""
-ACL_MAP =
-    {
-        ('<s3_method>', '<swift_method>', '<swift_resource>'):
-        {'Resource': '<check_resource>',
-         'Permission': '<check_permission>'},
-        ...
-    }
-
-s3_method: Method of S3 Request from user to swift3
-swift_method: Method of Swift Request from swift3 to swift
-swift_resource: Resource of Swift Request from swift3 to swift
-check_resource: <container/object>
-check_permission: <OWNER/READ/WRITE/READ_ACP/WRITE_ACP>
-"""
-ACL_MAP = {
-    # HEAD Bucket
-    ('HEAD', 'HEAD', 'container'):
-    {'Permission': 'READ'},
-    # GET Service
-    ('GET', 'HEAD', 'container'):
-    {'Permission': 'OWNER'},
-    # GET Bucket, List Parts, List Multipart Upload
-    ('GET', 'GET', 'container'):
-    {'Permission': 'READ'},
-    # PUT Object, PUT Object Copy
-    ('PUT', 'HEAD', 'container'):
-    {'Permission': 'WRITE'},
-    # Complete Multipart Upload
-    ('POST', 'GET', 'container'):
-    {'Permission': 'WRITE'},
-    # Initiate Multipart Upload
-    ('POST', 'PUT', 'container'):
-    {'Permission': 'WRITE'},
-    # Abort Multipart Upload
-    ('DELETE', 'HEAD', 'container'):
-    {'Permission': 'WRITE'},
-    # DELETE Bucket
-    ('DELETE', 'DELETE', 'container'):
-    {'Permission': 'OWNER'},
-    # HEAD Object
-    ('HEAD', 'HEAD', 'object'):
-    {'Permission': 'READ'},
-    # GET Object
-    ('GET', 'GET', 'object'):
-    {'Permission': 'READ'},
-    # PUT Object, PUT Object Copy
-    ('PUT', 'HEAD', 'object'):
-    {'Permission': 'WRITE'},
-    # Upload Part
-    ('PUT', 'PUT', 'object'):
-    {'Resource': 'container',
-     'Permission': 'WRITE'},
-    # Delete Object
-    ('DELETE', 'DELETE', 'object'):
-    {'Resource': 'container',
-     'Permission': 'WRITE'},
-    # DELETE Multiple Objects
-    ('POST', 'HEAD', 'container'):
-    {'Permission': 'WRITE'},
-}
+        # possible to skip recalling get_resposne_acl if resp is not
+        # None (e.g. HEAD)
+        if resp:
+            return resp
+        return self.get_acl_response(app, method, container, obj,
+                                     headers, body, query)

swift3/test/unit/test_acl_handlers.py

@@ -0,0 +1,42 @@
+# Copyright (c) 2014 OpenStack Foundation
+#
+# 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.
+
+import unittest
+
+from swift3.acl_handlers import S3AclHandler, BucketAclHandler, \
+    ObjectAclHandler, BaseAclHandler, PartAclHandler, UploadAclHandler, \
+    UploadsAclHandler, get_acl_handler
+
+
+class TestAclHandlers(unittest.TestCase):
+    def test_get_acl_handler(self):
+        expected_handlers = (('Bucket', BucketAclHandler),
+                             ('Object', ObjectAclHandler),
+                             ('S3Acl', S3AclHandler),
+                             ('Part', PartAclHandler),
+                             ('Upload', UploadAclHandler),
+                             ('Uploads', UploadsAclHandler),
+                             ('Foo', BaseAclHandler))
+        for name, expected in expected_handlers:
+            handler = get_acl_handler(name)
+            self.assertTrue(issubclass(handler, expected))
+
+    def test_handle_acl(self):
+        # we have already have tests for s3_acl checking at test_s3_acl.py
+        pass
+
+
+if __name__ == '__main__':
+    unittest.main()

swift3/test/unit/test_middleware.py

@@ -209,6 +209,12 @@ class TestSwift3Middleware(Swift3TestCase):
         self.assertEquals(status.split()[0], '200')
 
     def test_token_generation(self):
+        self.swift.register('HEAD', '/v1/AUTH_test/bucket+segments/'
+                                    'object/123456789abcdef',
+                            swob.HTTPOk, {}, None)
+        self.swift.register('PUT', '/v1/AUTH_test/bucket+segments/'
+                                   'object/123456789abcdef/1',
+                            swob.HTTPCreated, {}, None)
         req = Request.blank('/bucket/object?uploadId=123456789abcdef'
                             '&partNumber=1',
                             environ={'REQUEST_METHOD': 'PUT'})

swift3/test/unit/test_request.py

@@ -23,7 +23,7 @@ from swift3.subresource import ACL, User, Owner, Grant, encode_acl
 from swift3.test.unit.test_middleware import Swift3TestCase
 from swift3.cfg import CONF
 from swift3.request import Request as S3_Request
-from swift3.request import S3ACLRequest
+from swift3.request import S3AclRequest
 
 
 Fake_ACL_MAP = {
@@ -78,8 +78,8 @@ class TestRequest(Swift3TestCase):
     def tearDown(self):
         CONF.s3_acl = False
 
-    @patch('swift3.request.ACL_MAP', Fake_ACL_MAP)
-    @patch('swift3.request.S3ACLRequest.authenticate', lambda x, y: None)
+    @patch('swift3.acl_handlers.ACL_MAP', Fake_ACL_MAP)
+    @patch('swift3.request.S3AclRequest.authenticate', lambda x, y: None)
     def _test_get_response(self, method, container='bucket', obj=None,
                            permission=None, skip_check=False,
                            req_klass=S3_Request):
@@ -87,18 +87,16 @@ class TestRequest(Swift3TestCase):
         req = Request.blank(path,
                             environ={'REQUEST_METHOD': method},
                             headers={'Authorization': 'AWS test:tester:hmac'})
-        if issubclass(req_klass, S3ACLRequest):
+        if issubclass(req_klass, S3AclRequest):
             s3_req = req_klass(req.environ, MagicMock())
         else:
             s3_req = req_klass(req.environ)
-        # target = 'swift3.request.%s._get_response' % req_klass.__name__
         with nested(patch('swift3.request.Request._get_response'),
                     patch('swift3.subresource.ACL.check_permission')) \
                 as (mock_get_resp, m_check_permission):
             mock_get_resp.return_value = FakeResponse(CONF.s3_acl)
             return mock_get_resp, m_check_permission,\
-                s3_req.get_response(self.swift3, permission=permission,
-                                    skip_check=skip_check)
+                s3_req.get_response(self.swift3)
 
     def test_get_response_without_s3_acl(self):
         with patch('swift3.cfg.CONF.s3_acl', False):
@@ -109,46 +107,17 @@ class TestRequest(Swift3TestCase):
         self.assertEqual(mock_get_resp.call_count, 1)
         self.assertEqual(m_check_permission.call_count, 0)
 
-    def test_get_response_without_check_permission(self):
-        mock_get_resp, m_check_permission, s3_resp = \
-            self._test_get_response('HEAD', skip_check=True,
-                                    req_klass=S3ACLRequest)
-        self.assertTrue(s3_resp.bucket_acl is not None)
-        self.assertTrue(s3_resp.object_acl is not None)
-        self.assertEqual(mock_get_resp.call_count, 1)
-        self.assertEqual(m_check_permission.call_count, 0)
-
-    def test_get_response_with_permission_specified(self):
-        obj = 'object'
-        mock_get_resp, m_check_permission, s3_resp = \
-            self._test_get_response('GET', obj=obj,
-                                    permission='READ_ACP',
-                                    req_klass=S3ACLRequest)
-        self.assertTrue(s3_resp.bucket_acl is not None)
-        self.assertTrue(s3_resp.object_acl is not None)
-        self.assertEqual(mock_get_resp.call_count, 2)
-        args, kargs = mock_get_resp.call_args_list[0]
-        get_resp_obj = args[3]
-        self.assertEqual(get_resp_obj, obj)
-        self.assertEqual(m_check_permission.call_count, 1)
-        args, kargs = m_check_permission.call_args
-        permission = args[1]
-        self.assertEqual(permission, 'READ_ACP')
-
     def test_get_response_without_match_ACL_MAP(self):
-        mock_get_resp, m_check_permission, s3_resp = \
-            self._test_get_response('POST',
-                                    req_klass=S3ACLRequest)
-        self.assertTrue(s3_resp.bucket_acl is not None)
-        self.assertTrue(s3_resp.object_acl is not None)
-        self.assertEqual(mock_get_resp.call_count, 1)
-        self.assertEqual(m_check_permission.call_count, 0)
+        with self.assertRaises(Exception) as e:
+            self._test_get_response('POST', req_klass=S3AclRequest)
+        self.assertEquals(e.exception.message,
+                          'No permission to be checked exists')
 
     def test_get_response_without_duplication_HEAD_request(self):
         obj = 'object'
         mock_get_resp, m_check_permission, s3_resp = \
             self._test_get_response('HEAD', obj=obj,
-                                    req_klass=S3ACLRequest)
+                                    req_klass=S3AclRequest)
         self.assertTrue(s3_resp.bucket_acl is not None)
         self.assertTrue(s3_resp.object_acl is not None)
         self.assertEqual(mock_get_resp.call_count, 1)
@@ -164,7 +133,7 @@ class TestRequest(Swift3TestCase):
         obj = 'object'
         mock_get_resp, m_check_permission, s3_resp = \
             self._test_get_response('GET', obj=obj,
-                                    req_klass=S3ACLRequest)
+                                    req_klass=S3AclRequest)
         self.assertTrue(s3_resp.bucket_acl is not None)
         self.assertTrue(s3_resp.object_acl is not None)
         self.assertEqual(mock_get_resp.call_count, 2)
@@ -179,17 +148,18 @@ class TestRequest(Swift3TestCase):
     def test_get_response_with_check_container_permission(self):
         mock_get_resp, m_check_permission, s3_resp = \
             self._test_get_response('GET',
-                                    req_klass=S3ACLRequest)
+                                    req_klass=S3AclRequest)
         self.assertTrue(s3_resp.bucket_acl is not None)
         self.assertTrue(s3_resp.object_acl is not None)
         self.assertEqual(mock_get_resp.call_count, 2)
         args, kargs = mock_get_resp.call_args_list[0]
         get_resp_obj = args[3]
-        self.assertTrue(get_resp_obj is None)
+        self.assertTrue(get_resp_obj is '')
         self.assertEqual(m_check_permission.call_count, 1)
         args, kargs = m_check_permission.call_args
         permission = args[1]
         self.assertEqual(permission, 'READ')
 
+
 if __name__ == '__main__':
     unittest.main()