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

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)