Merge "s3api: Allow CORS preflight requests"

7 files changed, 161 insertions, 11 deletions

diff --git a/doc/saio/swift/proxy-server.conf b/doc/saio/swift/proxy-server.conf
index 57a054087..124b36c62 100644
--- a/doc/saio/swift/proxy-server.conf
+++ b/doc/saio/swift/proxy-server.conf
@@ -92,6 +92,7 @@ use = egg:swift#symlink
 use = egg:swift#s3api
 s3_acl = yes
 check_bucket_owner = yes
+cors_preflight_allow_origin = *
 
 # Example to create root secret: `openssl rand -base64 32`
 [filter:keymaster]
diff --git a/docker/rootfs/etc/swift/proxy-server.conf b/docker/rootfs/etc/swift/proxy-server.conf
index a964f1520..8189cb7f2 100644
--- a/docker/rootfs/etc/swift/proxy-server.conf
+++ b/docker/rootfs/etc/swift/proxy-server.conf
@@ -82,6 +82,7 @@ use = egg:swift#symlink
 # To enable, add the s3api middleware to the pipeline before tempauth
 [filter:s3api]
 use = egg:swift#s3api
+cors_preflight_allow_origin = *
 
 # Example to create root secret: `openssl rand -base64 32`
 [filter:keymaster]
diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample
index b474b27a3..434ed9c62 100644
--- a/etc/proxy-server.conf-sample
+++ b/etc/proxy-server.conf-sample
@@ -629,6 +629,12 @@ use = egg:swift#s3api
 # AWS allows clock skew up to 15 mins; note that older versions of swift/swift3
 # allowed at most 5 mins.
 # allowable_clock_skew = 900
+#
+# CORS preflight requests don't contain enough information for us to
+# identify the account that should be used for the real request, so
+# the allowed origins must be set cluster-wide. (default: blank; all
+# preflight requests will be denied)
+# cors_preflight_allow_origin =
 
 # You can override the default log routing for this filter here:
 # log_name = s3api
diff --git a/swift/common/middleware/s3api/s3api.py b/swift/common/middleware/s3api/s3api.py
index c5d805358..7b9ad0778 100644
--- a/swift/common/middleware/s3api/s3api.py
+++ b/swift/common/middleware/s3api/s3api.py
@@ -157,7 +157,7 @@ from swift.common.middleware.s3api.s3response import ErrorResponse, \
     InternalError, MethodNotAllowed, S3ResponseBase, S3NotImplemented
 from swift.common.utils import get_logger, register_swift_info, \
     config_true_value, config_positive_int_value, split_path, \
-    closing_if_possible
+    closing_if_possible, list_from_csv
 from swift.common.middleware.s3api.utils import Config
 from swift.common.middleware.s3api.acl_handlers import get_acl_handler
 
@@ -277,12 +277,43 @@ class S3ApiMiddleware(object):
             wsgi_conf.get('min_segment_size', 5242880))
         self.conf.allowable_clock_skew = config_positive_int_value(
             wsgi_conf.get('allowable_clock_skew', 15 * 60))
+        self.conf.cors_preflight_allow_origin = list_from_csv(wsgi_conf.get(
+            'cors_preflight_allow_origin', ''))
+        if '*' in self.conf.cors_preflight_allow_origin and \
+                len(self.conf.cors_preflight_allow_origin) > 1:
+            raise ValueError('if cors_preflight_allow_origin should include '
+                             'all domains, * must be the only entry')
 
         self.logger = get_logger(
             wsgi_conf, log_route=wsgi_conf.get('log_name', 's3api'))
         self.check_pipeline(wsgi_conf)
 
     def __call__(self, env, start_response):
+        origin = env.get('HTTP_ORIGIN')
+        acrh = env.get('HTTP_ACCESS_CONTROL_REQUEST_HEADERS', '').lower()
+        if self.conf.cors_preflight_allow_origin and origin and \
+                env['REQUEST_METHOD'] == 'OPTIONS' and \
+                'authorization' in acrh and \
+                not env['PATH_INFO'].startswith(('/v1/', '/v1.0/')):
+            # I guess it's likely going to be an S3 request? *shrug*
+            if self.conf.cors_preflight_allow_origin != ['*'] and \
+                    origin not in self.conf.cors_preflight_allow_origin:
+                start_response('401 Unauthorized', [
+                    ('Allow', 'GET, HEAD, PUT, POST, DELETE, OPTIONS'),
+                ])
+                return [b'']
+
+            start_response('200 OK', [
+                ('Allow', 'GET, HEAD, PUT, POST, DELETE, OPTIONS'),
+                ('Access-Control-Allow-Origin', origin),
+                ('Access-Control-Allow-Methods',
+                 'GET, HEAD, PUT, POST, DELETE, OPTIONS'),
+                ('Access-Control-Allow-Headers',
+                 ', '.join(set(list_from_csv(acrh)))),
+                ('Vary', 'Origin, Access-Control-Request-Headers'),
+            ])
+            return [b'']
+
         try:
             req_class = get_request_class(env, self.conf.s3_acl)
             req = req_class(env, self.app, self.conf)
diff --git a/test/cors/test-s3-obj.js b/test/cors/test-s3-obj.js
index 35dd19815..dfa119ef9 100644
--- a/test/cors/test-s3-obj.js
+++ b/test/cors/test-s3-obj.js
@@ -133,28 +133,69 @@ function makeTests (params) {
         Bucket: 'private-with-cors',
         Key: 'obj'
       })
-        .then(CorsBlocked)],  // Pre-flight failed
-    ['PUT',
-      () => MakeS3Request(service, 'putObject', {
-        Bucket: 'private-with-cors',
-        Key: 'put-target',
-        Body: 'test'
-      })
-        .then(CorsBlocked)],  // Pre-flight failed
+        .then(HasStatus(200, 'OK'))
+        .then(CheckS3Headers)
+        .then(HasHeaders(['x-amz-meta-mtime']))
+        .then(DoesNotHaveHeaders(['X-Object-Meta-Mtime']))
+        .then(HasHeaders({
+          'Content-Type': 'application/octet-stream',
+          Etag: '"0f343b0931126a20f133d67c2b018a3b"'
+        }))
+        .then(BodyHasLength(1024))],
+    ['PUT then DELETE',
+      () => Promise.resolve('put-target-' + Math.random()).then((objectName) => {
+        return MakeS3Request(service, 'putObject', {
+          Bucket: 'private-with-cors',
+          Key: objectName,
+          Body: 'test'
+        })
+          .then(HasStatus(200, 'OK'))
+          .then(CheckS3Headers)
+          .then(HasHeaders({
+            'Content-Type': 'text/html; charset=UTF-8',
+            Etag: '"098f6bcd4621d373cade4e832627b4f6"'
+          }))
+          .then(HasNoBody)
+          .then((resp) => {
+            return MakeS3Request(service, 'deleteObject', {
+              Bucket: 'private-with-cors',
+              Key: objectName
+            })
+          })
+          .then(HasStatus(204, 'No Content'))
+          .then(CheckTransactionIdHeaders)
+          .then(HasNoBody)
+      })],
     ['GET If-Match matching',
       () => MakeS3Request(service, 'getObject', {
         Bucket: 'private-with-cors',
         Key: 'obj',
         IfMatch: '0f343b0931126a20f133d67c2b018a3b'
       })
-        .then(CorsBlocked)],  // Pre-flight failed
+        .then(HasStatus(200, 'OK'))
+        .then(CheckS3Headers)
+        .then(HasHeaders(['x-amz-meta-mtime']))
+        .then(DoesNotHaveHeaders(['X-Object-Meta-Mtime']))
+        .then(HasHeaders({
+          'Content-Type': 'application/octet-stream',
+          Etag: '"0f343b0931126a20f133d67c2b018a3b"'
+        }))
+        .then(BodyHasLength(1024))],
     ['GET Range',
       () => MakeS3Request(service, 'getObject', {
         Bucket: 'private-with-cors',
         Key: 'obj',
         Range: 'bytes=100-199'
       })
-        .then(CorsBlocked)],  // Pre-flight failed
+        .then(HasStatus(206, 'Partial Content'))
+        .then(CheckS3Headers)
+        .then(HasHeaders(['x-amz-meta-mtime']))
+        .then(DoesNotHaveHeaders(['X-Object-Meta-Mtime']))
+        .then(HasHeaders({
+          'Content-Type': 'application/octet-stream',
+          Etag: '"0f343b0931126a20f133d67c2b018a3b"'
+        }))
+        .then(BodyHasLength(100))]
   ]
 }
 
diff --git a/test/unit/common/middleware/s3api/test_obj.py b/test/unit/common/middleware/s3api/test_obj.py
index fcf3b78c9..6b6d0ce76 100644
--- a/test/unit/common/middleware/s3api/test_obj.py
+++ b/test/unit/common/middleware/s3api/test_obj.py
@@ -1724,6 +1724,47 @@ class TestS3ApiObj(S3ApiTestCase):
             'test:write', 'READ', src_path='')
         self.assertEqual(status.split()[0], '400')
 
+    def test_cors_preflight(self):
+        req = Request.blank(
+            '/bucket/cors-object',
+            environ={'REQUEST_METHOD': 'OPTIONS'},
+            headers={'Origin': 'http://example.com',
+                     'Access-Control-Request-Method': 'GET',
+                     'Access-Control-Request-Headers': 'authorization'})
+        self.s3api.conf.cors_preflight_allow_origin = ['*']
+        status, headers, body = self.call_s3api(req)
+        self.assertEqual(status, '200 OK')
+        self.assertDictEqual(headers, {
+            'Allow': 'GET, HEAD, PUT, POST, DELETE, OPTIONS',
+            'Access-Control-Allow-Origin': 'http://example.com',
+            'Access-Control-Allow-Methods': ('GET, HEAD, PUT, POST, DELETE, '
+                                             'OPTIONS'),
+            'Access-Control-Allow-Headers': 'authorization',
+            'Vary': 'Origin, Access-Control-Request-Headers',
+        })
+
+        # test more allow_origins
+        self.s3api.conf.cors_preflight_allow_origin = ['http://example.com',
+                                                       'http://other.com']
+        status, headers, body = self.call_s3api(req)
+        self.assertEqual(status, '200 OK')
+        self.assertDictEqual(headers, {
+            'Allow': 'GET, HEAD, PUT, POST, DELETE, OPTIONS',
+            'Access-Control-Allow-Origin': 'http://example.com',
+            'Access-Control-Allow-Methods': ('GET, HEAD, PUT, POST, DELETE, '
+                                             'OPTIONS'),
+            'Access-Control-Allow-Headers': 'authorization',
+            'Vary': 'Origin, Access-Control-Request-Headers',
+        })
+
+        # Wrong protocol
+        self.s3api.conf.cors_preflight_allow_origin = ['https://example.com']
+        status, headers, body = self.call_s3api(req)
+        self.assertEqual(status, '401 Unauthorized')
+        self.assertEqual(headers, {
+            'Allow': 'GET, HEAD, PUT, POST, DELETE, OPTIONS',
+        })
+
     def test_cors_headers(self):
         # note: Access-Control-Allow-Methods would normally be expected in
         # response to an OPTIONS request but its included here in GET/PUT tests
diff --git a/test/unit/common/middleware/s3api/test_s3api.py b/test/unit/common/middleware/s3api/test_s3api.py
index e530e7071..fb9b0f4d8 100644
--- a/test/unit/common/middleware/s3api/test_s3api.py
+++ b/test/unit/common/middleware/s3api/test_s3api.py
@@ -118,6 +118,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
             'min_segment_size': 5242880,
             'multi_delete_concurrency': 2,
             's3_acl': False,
+            'cors_preflight_allow_origin': [],
         })
         s3api = S3ApiMiddleware(None, {})
         self.assertEqual(expected, s3api.conf)
@@ -140,10 +141,38 @@ class TestS3ApiMiddleware(S3ApiTestCase):
             'min_segment_size': 1000000,
             'multi_delete_concurrency': 1,
             's3_acl': True,
+            'cors_preflight_allow_origin': 'foo.example.com,bar.example.com',
         }
         s3api = S3ApiMiddleware(None, conf)
+        conf['cors_preflight_allow_origin'] = \
+            conf['cors_preflight_allow_origin'].split(',')
         self.assertEqual(conf, s3api.conf)
 
+        # test allow_origin list with a '*' fails.
+        conf = {
+            'storage_domain': 'somewhere',
+            'location': 'us-west-1',
+            'force_swift_request_proxy_log': True,
+            'dns_compliant_bucket_names': False,
+            'allow_multipart_uploads': False,
+            'allow_no_owner': True,
+            'allowable_clock_skew': 300,
+            'auth_pipeline_check': False,
+            'check_bucket_owner': True,
+            'max_bucket_listing': 500,
+            'max_multi_delete_objects': 600,
+            'max_parts_listing': 70,
+            'max_upload_part_num': 800,
+            'min_segment_size': 1000000,
+            'multi_delete_concurrency': 1,
+            's3_acl': True,
+            'cors_preflight_allow_origin': 'foo.example.com,bar.example.com,*',
+        }
+        with self.assertRaises(ValueError) as ex:
+            S3ApiMiddleware(None, conf)
+        self.assertIn("if cors_preflight_allow_origin should include all "
+                      "domains, * must be the only entry", str(ex.exception))
+
         def check_bad_positive_ints(**kwargs):
             bad_conf = dict(conf, **kwargs)
             self.assertRaises(ValueError, S3ApiMiddleware, None, bad_conf)