Switched StrOpt to ListOpt in CORS allowed_origins

This patch switches the 'allowed_origin' CORS configuration option
from a single string to an array of strings. This will let you
configure multiple domains simultaneously with the same options,
without having to add additional configuration blocks.

By doing this, pastedeploy users will no longer have to configure
mulitple filters if they wish to grant access to more than one
domain.

Change-Id: Ie2e57b76717604f701daa16ebf8ffa8c06835e3c

2 files changed, 73 insertions, 31 deletions

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)