Merge "Switched StrOpt to ListOpt in CORS allowed_origins"

3 files changed, 83 insertions, 40 deletions

diff --git a/doc/source/cors.rst b/doc/source/cors.rst
index 368b248..99e5dc0 100644
--- a/doc/source/cors.rst
+++ b/doc/source/cors.rst
@@ -49,18 +49,19 @@ In your application's config file, then include a default configuration block
 something like this::
 
     [cors]
-    allowed_origin=https://website.example.com:443
+    allowed_origin=https://website.example.com:443,https://website2.example.com:443
     max_age=3600
     allow_methods=GET,POST,PUT,DELETE
     allow_headers=Content-Type,Cache-Control,Content-Language,Expires,Last-Modified,Pragma,X-Custom-Header
     expose_headers=Content-Type,Cache-Control,Content-Language,Expires,Last-Modified,Pragma,X-Custom-Header
 
-This middleware permits you to define multiple `allowed_origin`'s. To express
-this in your configuration file, first begin with a `[cors]` group as above,
-into which you place your default configuration values. Then add as many
-additional configuration groups as necessary, naming them `[cors.something]`
-(each name must be unique). The purpose of the suffix to `cors.` is
-legibility, we recommend using a reasonable human-readable string::
+This middleware permits you to override the rules for multiple
+`allowed_origin`'s. To express this in your configuration file, first begin
+with a `[cors]` group as above, into which you place your default
+configuration values. Then add as many additional configuration groups as
+necessary, naming them `[cors.something]` (each name must be unique). The
+purpose of the suffix to `cors.` is legibility, we recommend using a
+reasonable human-readable string::
 
     [cors.ironic_webclient]
     # CORS Configuration for a hypothetical ironic webclient, which overrides
@@ -94,11 +95,11 @@ Configuration for pastedeploy
 -----------------------------
 
 If your application is using pastedeploy, the following configuration block
-will add CORS support. To add multiple domains, simply add another filter.::
+will add CORS support.::
 
     [filter:cors]
     paste.filter_factory = oslo_middleware.cors:filter_factory
-    allowed_origin=https://website.example.com:443
+    allowed_origin=https://website.example.com:443,https://website2.example.com:443
     max_age=3600
     allow_methods=GET,POST,PUT,DELETE
     allow_headers=Content-Type,Cache-Control,Content-Language,Expires,Last-Modified,Pragma,X-Custom-Header
diff --git a/oslo_middleware/cors.py b/oslo_middleware/cors.py
index 3735d9c..ac86bc1 100644
--- a/oslo_middleware/cors.py
+++ b/oslo_middleware/cors.py
@@ -25,10 +25,10 @@ import webob.response
 LOG = logging.getLogger(__name__)
 
 CORS_OPTS = [
-    cfg.StrOpt('allowed_origin',
-               default=None,
-               help='Indicate whether this resource may be shared with the '
-                    'domain received in the requests "origin" header.'),
+    cfg.ListOpt('allowed_origin',
+                default=None,
+                help='Indicate whether this resource may be shared with the '
+                     'domain received in the requests "origin" header.'),
     cfg.BoolOpt('allow_credentials',
                 default=True,
                 help='Indicate that the actual request can include user '
@@ -180,19 +180,20 @@ class CORS(base.ConfigurableMiddleware):
         :param allow_headers: List of HTTP headers to permit from the client.
         :return:
         '''
-
-        if allowed_origin in self.allowed_origins:
-            LOG.warn('Allowed origin [%s] already exists, skipping' % (
-                allowed_origin,))
-            return
-
-        self.allowed_origins[allowed_origin] = {
-            'allow_credentials': allow_credentials,
-            'expose_headers': expose_headers,
-            'max_age': max_age,
-            'allow_methods': allow_methods,
-            'allow_headers': allow_headers
-        }
+        for origin in allowed_origin:
+
+            if origin in self.allowed_origins:
+                LOG.warn('Allowed origin [%s] already exists, skipping' % (
+                    allowed_origin,))
+                continue
+
+            self.allowed_origins[origin] = {
+                'allow_credentials': allow_credentials,
+                'expose_headers': expose_headers,
+                'max_age': max_age,
+                'allow_methods': allow_methods,
+                'allow_headers': allow_headers
+            }
 
     def set_latent(self, allow_headers=None, allow_methods=None,
                    expose_headers=None):
diff --git a/oslo_middleware/tests/test_cors.py b/oslo_middleware/tests/test_cors.py
index 7037459..e47c5d8 100644
--- a/oslo_middleware/tests/test_cors.py
+++ b/oslo_middleware/tests/test_cors.py
@@ -136,6 +136,18 @@ class CORSTestFilterFactory(test_base.BaseTestCase):
         self.assertEqual(['GET'], config['allow_methods'])
         self.assertEqual([], config['allow_headers'])
 
+    def test_filter_factory_multiorigin(self):
+        self.useFixture(fixture.Config()).conf([])
+
+        # Test a valid filter.
+        filter = cors.filter_factory(None,
+                                     allowed_origin='http://valid.example.com,'
+                                                    'http://other.example.com')
+        application = filter(test_application)
+
+        self.assertIn('http://valid.example.com', application.allowed_origins)
+        self.assertIn('http://other.example.com', application.allowed_origins)
+
     def test_no_origin_fail(self):
         '''Assert that a filter factory with no allowed_origin fails.'''
         self.assertRaises(TypeError,
@@ -220,6 +232,10 @@ class CORSRegularRequestTest(CORSTestBase):
                                allowed_origin='http://all.example.com',
                                allow_methods='GET,PUT,POST,DELETE,HEAD')
 
+        config.load_raw_values(group='cors.duplicate',
+                               allowed_origin='http://domain1.example.com,'
+                                              'http://domain2.example.com')
+
         # Now that the config is set up, create our application.
         self.application = cors.CORS(test_application, cfg.CONF)
 
@@ -228,7 +244,7 @@ class CORSRegularRequestTest(CORSTestBase):
 
         # Confirm global configuration
         gc = cfg.CONF.cors
-        self.assertEqual(gc.allowed_origin, 'http://valid.example.com')
+        self.assertEqual(gc.allowed_origin, ['http://valid.example.com'])
         self.assertEqual(gc.allow_credentials, False)
         self.assertEqual(gc.expose_headers, [])
         self.assertEqual(gc.max_age, None)
@@ -237,7 +253,7 @@ class CORSRegularRequestTest(CORSTestBase):
 
         # Confirm credentials overrides.
         cc = cfg.CONF['cors.credentials']
-        self.assertEqual(cc.allowed_origin, 'http://creds.example.com')
+        self.assertEqual(cc.allowed_origin, ['http://creds.example.com'])
         self.assertEqual(cc.allow_credentials, True)
         self.assertEqual(cc.expose_headers, gc.expose_headers)
         self.assertEqual(cc.max_age, gc.max_age)
@@ -246,7 +262,7 @@ class CORSRegularRequestTest(CORSTestBase):
 
         # Confirm exposed-headers overrides.
         ec = cfg.CONF['cors.exposed-headers']
-        self.assertEqual(ec.allowed_origin, 'http://headers.example.com')
+        self.assertEqual(ec.allowed_origin, ['http://headers.example.com'])
         self.assertEqual(ec.allow_credentials, gc.allow_credentials)
         self.assertEqual(ec.expose_headers, ['X-Header-1', 'X-Header-2'])
         self.assertEqual(ec.max_age, gc.max_age)
@@ -255,7 +271,7 @@ class CORSRegularRequestTest(CORSTestBase):
 
         # Confirm cached overrides.
         chc = cfg.CONF['cors.cached']
-        self.assertEqual(chc.allowed_origin, 'http://cached.example.com')
+        self.assertEqual(chc.allowed_origin, ['http://cached.example.com'])
         self.assertEqual(chc.allow_credentials, gc.allow_credentials)
         self.assertEqual(chc.expose_headers, gc.expose_headers)
         self.assertEqual(chc.max_age, 3600)
@@ -264,7 +280,7 @@ class CORSRegularRequestTest(CORSTestBase):
 
         # Confirm get-only overrides.
         goc = cfg.CONF['cors.get-only']
-        self.assertEqual(goc.allowed_origin, 'http://get.example.com')
+        self.assertEqual(goc.allowed_origin, ['http://get.example.com'])
         self.assertEqual(goc.allow_credentials, gc.allow_credentials)
         self.assertEqual(goc.expose_headers, gc.expose_headers)
         self.assertEqual(goc.max_age, gc.max_age)
@@ -273,7 +289,7 @@ class CORSRegularRequestTest(CORSTestBase):
 
         # Confirm all-methods overrides.
         ac = cfg.CONF['cors.all-methods']
-        self.assertEqual(ac.allowed_origin, 'http://all.example.com')
+        self.assertEqual(ac.allowed_origin, ['http://all.example.com'])
         self.assertEqual(ac.allow_credentials, gc.allow_credentials)
         self.assertEqual(ac.expose_headers, gc.expose_headers)
         self.assertEqual(ac.max_age, gc.max_age)
@@ -281,6 +297,16 @@ class CORSRegularRequestTest(CORSTestBase):
                          ['GET', 'PUT', 'POST', 'DELETE', 'HEAD'])
         self.assertEqual(ac.allow_headers, gc.allow_headers)
 
+        # Confirm duplicate domains.
+        ac = cfg.CONF['cors.duplicate']
+        self.assertEqual(ac.allowed_origin, ['http://domain1.example.com',
+                                             'http://domain2.example.com'])
+        self.assertEqual(ac.allow_credentials, gc.allow_credentials)
+        self.assertEqual(ac.expose_headers, gc.expose_headers)
+        self.assertEqual(ac.max_age, gc.max_age)
+        self.assertEqual(ac.allow_methods, gc.allow_methods)
+        self.assertEqual(ac.allow_headers, gc.allow_headers)
+
     def test_no_origin_header(self):
         """CORS Specification Section 6.1.1
 
@@ -352,6 +378,21 @@ class CORSRegularRequestTest(CORSTestBase):
                                     allow_credentials=None,
                                     expose_headers=None)
 
+        # Test valid header from list of duplicates.
+        for method in self.methods:
+            request = webob.Request.blank('/')
+            request.method = method
+            request.headers['Origin'] = 'http://domain2.example.com'
+            response = request.get_response(self.application)
+            self.assertCORSResponse(response,
+                                    status='200 OK',
+                                    allow_origin='http://domain2.example.com',
+                                    max_age=None,
+                                    allow_methods=None,
+                                    allow_headers=None,
+                                    allow_credentials=None,
+                                    expose_headers=None)
+
     def test_supports_credentials(self):
         """CORS Specification Section 6.1.3
 
@@ -488,7 +529,7 @@ class CORSPreflightRequestTest(CORSTestBase):
 
         # Confirm global configuration
         gc = cfg.CONF.cors
-        self.assertEqual(gc.allowed_origin, 'http://valid.example.com')
+        self.assertEqual(gc.allowed_origin, ['http://valid.example.com'])
         self.assertEqual(gc.allow_credentials, False)
         self.assertEqual(gc.expose_headers, [])
         self.assertEqual(gc.max_age, None)
@@ -497,7 +538,7 @@ class CORSPreflightRequestTest(CORSTestBase):
 
         # Confirm credentials overrides.
         cc = cfg.CONF['cors.credentials']
-        self.assertEqual(cc.allowed_origin, 'http://creds.example.com')
+        self.assertEqual(cc.allowed_origin, ['http://creds.example.com'])
         self.assertEqual(cc.allow_credentials, True)
         self.assertEqual(cc.expose_headers, gc.expose_headers)
         self.assertEqual(cc.max_age, gc.max_age)
@@ -506,7 +547,7 @@ class CORSPreflightRequestTest(CORSTestBase):
 
         # Confirm exposed-headers overrides.
         ec = cfg.CONF['cors.exposed-headers']
-        self.assertEqual(ec.allowed_origin, 'http://headers.example.com')
+        self.assertEqual(ec.allowed_origin, ['http://headers.example.com'])
         self.assertEqual(ec.allow_credentials, gc.allow_credentials)
         self.assertEqual(ec.expose_headers, ['X-Header-1', 'X-Header-2'])
         self.assertEqual(ec.max_age, gc.max_age)
@@ -515,7 +556,7 @@ class CORSPreflightRequestTest(CORSTestBase):
 
         # Confirm cached overrides.
         chc = cfg.CONF['cors.cached']
-        self.assertEqual(chc.allowed_origin, 'http://cached.example.com')
+        self.assertEqual(chc.allowed_origin, ['http://cached.example.com'])
         self.assertEqual(chc.allow_credentials, gc.allow_credentials)
         self.assertEqual(chc.expose_headers, gc.expose_headers)
         self.assertEqual(chc.max_age, 3600)
@@ -524,7 +565,7 @@ class CORSPreflightRequestTest(CORSTestBase):
 
         # Confirm get-only overrides.
         goc = cfg.CONF['cors.get-only']
-        self.assertEqual(goc.allowed_origin, 'http://get.example.com')
+        self.assertEqual(goc.allowed_origin, ['http://get.example.com'])
         self.assertEqual(goc.allow_credentials, gc.allow_credentials)
         self.assertEqual(goc.expose_headers, gc.expose_headers)
         self.assertEqual(goc.max_age, gc.max_age)
@@ -533,7 +574,7 @@ class CORSPreflightRequestTest(CORSTestBase):
 
         # Confirm all-methods overrides.
         ac = cfg.CONF['cors.all-methods']
-        self.assertEqual(ac.allowed_origin, 'http://all.example.com')
+        self.assertEqual(ac.allowed_origin, ['http://all.example.com'])
         self.assertEqual(ac.allow_credentials, gc.allow_credentials)
         self.assertEqual(ac.expose_headers, gc.expose_headers)
         self.assertEqual(ac.max_age, gc.max_age)
@@ -1006,7 +1047,7 @@ class CORSTestWildcard(CORSTestBase):
 
         # Confirm global configuration
         gc = cfg.CONF.cors
-        self.assertEqual(gc.allowed_origin, 'http://default.example.com')
+        self.assertEqual(gc.allowed_origin, ['http://default.example.com'])
         self.assertEqual(gc.allow_credentials, True)
         self.assertEqual(gc.expose_headers, [])
         self.assertEqual(gc.max_age, None)
@@ -1016,7 +1057,7 @@ class CORSTestWildcard(CORSTestBase):
 
         # Confirm all-methods overrides.
         ac = cfg.CONF['cors.wildcard']
-        self.assertEqual(ac.allowed_origin, '*')
+        self.assertEqual(ac.allowed_origin, ['*'])
         self.assertEqual(gc.allow_credentials, True)
         self.assertEqual(ac.expose_headers, gc.expose_headers)
         self.assertEqual(ac.max_age, gc.max_age)