diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index 70b0d0cf68..2b53ba7a80 100644 --- a/swift/proxy/controllers/obj.py +++ b/swift/proxy/controllers/obj.py @@ -910,6 +910,10 @@ class ObjectController(Controller): req.acl = container_info['write_acl'] req.environ['swift_sync_key'] = container_info['sync_key'] object_versions = container_info['versions'] + if 'swift.authorize' in req.environ: + aresp = req.environ['swift.authorize'](req) + if aresp: + return aresp if object_versions: # this is a version manifest and needs to be handled differently object_versions = unquote(object_versions) @@ -980,11 +984,11 @@ class ObjectController(Controller): # remove 'X-If-Delete-At', since it is not for the older copy if 'X-If-Delete-At' in req.headers: del req.headers['X-If-Delete-At'] + if 'swift.authorize' in req.environ: + aresp = req.environ['swift.authorize'](req) + if aresp: + return aresp break - if 'swift.authorize' in req.environ: - aresp = req.environ['swift.authorize'](req) - if aresp: - return aresp if not containers: return HTTPNotFound(request=req) partition, nodes = obj_ring.get_nodes( diff --git a/test/functional/tests.py b/test/functional/tests.py index 931f3640a5..626880198f 100644 --- a/test/functional/tests.py +++ b/test/functional/tests.py @@ -2409,6 +2409,14 @@ class TestObjectVersioningEnv(object): cls.account = Account(cls.conn, tf.config.get('account', tf.config['username'])) + # Second connection for ACL tests + config2 = deepcopy(tf.config) + config2['account'] = tf.config['account2'] + config2['username'] = tf.config['username2'] + config2['password'] = tf.config['password2'] + cls.conn2 = Connection(config2) + cls.conn2.authenticate() + # avoid getting a prefix that stops halfway through an encoded # character prefix = Utils.create_name().decode("utf-8")[:10].encode("utf-8") @@ -2462,6 +2470,14 @@ class TestCrossPolicyObjectVersioningEnv(object): cls.account = Account(cls.conn, tf.config.get('account', tf.config['username'])) + # Second connection for ACL tests + config2 = deepcopy(tf.config) + config2['account'] = tf.config['account2'] + config2['username'] = tf.config['username2'] + config2['password'] = tf.config['password2'] + cls.conn2 = Connection(config2) + cls.conn2.authenticate() + # avoid getting a prefix that stops halfway through an encoded # character prefix = Utils.create_name().decode("utf-8")[:10].encode("utf-8") @@ -2496,6 +2512,15 @@ class TestObjectVersioning(Base): "Expected versioning_enabled to be True/False, got %r" % (self.env.versioning_enabled,)) + def tearDown(self): + super(TestObjectVersioning, self).tearDown() + try: + # delete versions first! + self.env.versions_container.delete_files() + self.env.container.delete_files() + except ResponseError: + pass + def test_overwriting(self): container = self.env.container versions_container = self.env.versions_container @@ -2555,6 +2580,33 @@ class TestObjectVersioning(Base): self.assertEqual(3, versions_container.info()['object_count']) self.assertEqual("112233", man_file.read()) + def test_versioning_check_acl(self): + container = self.env.container + versions_container = self.env.versions_container + versions_container.create(hdrs={'X-Container-Read': '.r:*,.rlistings'}) + + obj_name = Utils.create_name() + versioned_obj = container.file(obj_name) + versioned_obj.write("aaaaa") + self.assertEqual("aaaaa", versioned_obj.read()) + + versioned_obj.write("bbbbb") + self.assertEqual("bbbbb", versioned_obj.read()) + + # Use token from second account and try to delete the object + org_token = self.env.account.conn.storage_token + self.env.account.conn.storage_token = self.env.conn2.storage_token + try: + self.assertRaises(ResponseError, versioned_obj.delete) + finally: + self.env.account.conn.storage_token = org_token + + # Verify with token from first account + self.assertEqual("bbbbb", versioned_obj.read()) + + versioned_obj.delete() + self.assertEqual("aaaaa", versioned_obj.read()) + class TestObjectVersioningUTF8(Base2, TestObjectVersioning): set_up = False diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 39d637d8c5..41f0ea3c39 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -56,7 +56,7 @@ from swift.proxy.controllers.base import get_container_memcache_key, \ get_account_memcache_key, cors_validation import swift.proxy.controllers from swift.common.swob import Request, Response, HTTPUnauthorized, \ - HTTPException + HTTPException, HTTPForbidden from swift.common import storage_policy from swift.common.storage_policy import StoragePolicy, \ StoragePolicyCollection, POLICIES @@ -1615,6 +1615,7 @@ class TestObjectController(unittest.TestCase): ]) def test_DELETE_on_expired_versioned_object(self): methods = set() + authorize_call_count = [0] def test_connect(ipaddr, port, device, partition, method, path, headers=None, query_string=None): @@ -1640,6 +1641,10 @@ class TestObjectController(unittest.TestCase): for obj in object_list: yield obj + def fake_authorize(req): + authorize_call_count[0] += 1 + return None # allow the request + with save_globals(): controller = proxy_server.ObjectController(self.app, 'a', 'c', 'o') @@ -1651,7 +1656,8 @@ class TestObjectController(unittest.TestCase): 204, 204, 204, # delete for the pre-previous give_connect=test_connect) req = Request.blank('/v1/a/c/o', - environ={'REQUEST_METHOD': 'DELETE'}) + environ={'REQUEST_METHOD': 'DELETE', + 'swift.authorize': fake_authorize}) self.app.memcache.store = {} self.app.update_request(req) @@ -1661,6 +1667,67 @@ class TestObjectController(unittest.TestCase): ('PUT', '/a/c/o'), ('DELETE', '/a/foo/2')] self.assertEquals(set(exp_methods), (methods)) + self.assertEquals(authorize_call_count[0], 2) + + @patch_policies([ + StoragePolicy(0, 'zero', False, object_ring=FakeRing()), + StoragePolicy(1, 'one', True, object_ring=FakeRing()) + ]) + def test_denied_DELETE_of_versioned_object(self): + """ + Verify that a request with read access to a versions container + is unable to cause any write operations on the versioned container. + """ + methods = set() + authorize_call_count = [0] + + def test_connect(ipaddr, port, device, partition, method, path, + headers=None, query_string=None): + methods.add((method, path)) + + def fake_container_info(account, container, req): + return {'status': 200, 'sync_key': None, + 'meta': {}, 'cors': {'allow_origin': None, + 'expose_headers': None, + 'max_age': None}, + 'sysmeta': {}, 'read_acl': None, 'object_count': None, + 'write_acl': None, 'versions': 'foo', + 'partition': 1, 'bytes': None, 'storage_policy': '1', + 'nodes': [{'zone': 0, 'ip': '10.0.0.0', 'region': 0, + 'id': 0, 'device': 'sda', 'port': 1000}, + {'zone': 1, 'ip': '10.0.0.1', 'region': 1, + 'id': 1, 'device': 'sdb', 'port': 1001}, + {'zone': 2, 'ip': '10.0.0.2', 'region': 0, + 'id': 2, 'device': 'sdc', 'port': 1002}]} + + def fake_list_iter(container, prefix, env): + object_list = [{'name': '1'}, {'name': '2'}, {'name': '3'}] + for obj in object_list: + yield obj + + def fake_authorize(req): + # deny write access + authorize_call_count[0] += 1 + return HTTPForbidden(req) # allow the request + + with save_globals(): + controller = proxy_server.ObjectController(self.app, + 'a', 'c', 'o') + controller.container_info = fake_container_info + # patching _listing_iter simulates request being authorized + # to list versions container + controller._listing_iter = fake_list_iter + set_http_connect(give_connect=test_connect) + req = Request.blank('/v1/a/c/o', + environ={'REQUEST_METHOD': 'DELETE', + 'swift.authorize': fake_authorize}) + + self.app.memcache.store = {} + self.app.update_request(req) + resp = controller.DELETE(req) + self.assertEqual(403, resp.status_int) + self.assertFalse(methods, methods) + self.assertEquals(authorize_call_count[0], 1) def test_PUT_auto_content_type(self): with save_globals():