V2.9.3

CHANGELOG.rst

@@ -1,6 +1,17 @@
 Changelog
 =========
 
+In development
+--------------
+
+Fixed
+~~~~~
+
+* Fix improper CORS where request from an origin not listed in ``allowed_origins`` will be responded
+  with ``null`` for the ``Access-Control-Allow-Origin`` header. The fix returns the first of our
+  allowed origins if the requesting origin is not a supported origin. Reported by Barak Tawily.
+  (bug fix)
+
 2.9.2 - December 19, 2018
 -------------------------
 

Makefile

@@ -198,7 +198,7 @@ lint-api-spec: requirements .lint-api-spec
 generate-api-spec: requirements .generate-api-spec
 
 .PHONY: .generate-api-spec
-.generate-api-spec: .lint-api-spec
+.generate-api-spec:
 	@echo
 	@echo "================== Generate openapi.yaml file ===================="
 	@echo
@@ -364,12 +364,6 @@ requirements: virtualenv .sdist-requirements install-runners
 	# Install st2common package to load drivers defined in st2common setup.py
 	(cd st2common; ${ROOT_DIR}/$(VIRTUALENV_DIR)/bin/python setup.py develop)
 
-
-	# Note: We install prance here and not as part of any component
-	# requirements.txt because it has a conflict with our dependency (requires
-	# new version of requests) which we cant resolve at this moment
-	$(VIRTUALENV_DIR)/bin/pip install "prance==0.6.1"
-
 	# Install st2common to register metrics drivers
 	(cd ${ROOT_DIR}/st2common; ${ROOT_DIR}/$(VIRTUALENV_DIR)/bin/python setup.py develop)
 
@@ -789,7 +783,7 @@ debs:
 ci: ci-checks ci-unit ci-integration ci-mistral ci-packs-tests
 
 .PHONY: ci-checks
-ci-checks: compile .generated-files-check .pylint .flake8 .bandit .st2client-dependencies-check .st2common-circular-dependencies-check circle-lint-api-spec .rst-check
+ci-checks: compile .generated-files-check .pylint .flake8 .bandit .st2client-dependencies-check .st2common-circular-dependencies-check .rst-check
 
 .PHONY: ci-py3-unit
 ci-py3-unit:

st2api/tests/unit/controllers/v1/test_base.py

@@ -47,12 +47,30 @@ def test_additional_origin(self):
                          'http://dev')
 
     def test_wrong_origin(self):
+        # Invalid origin  (not specified in the config), we return first allowed origin specified
+        # in the config
         response = self.app.get('/', headers={
             'origin': 'http://xss'
         })
         self.assertEqual(response.status_int, 200)
-        self.assertEqual(response.headers['Access-Control-Allow-Origin'],
-                         'null')
+        self.assertEqual(response.headers.get('Access-Control-Allow-Origin'),
+                        'http://127.0.0.1:3000')
+
+        invalid_origins = [
+            'http://',
+            'https://',
+            'https://www.example.com',
+            'null',
+            '*'
+        ]
+
+        for origin in invalid_origins:
+            response = self.app.get('/', headers={
+                'origin': origin
+            })
+            self.assertEqual(response.status_int, 200)
+            self.assertEqual(response.headers.get('Access-Control-Allow-Origin'),
+                            'http://127.0.0.1:3000')
 
     def test_wildcard_origin(self):
         try:

st2common/st2common/middleware/cors.py

@@ -66,7 +66,7 @@ def custom_start_response(status, headers, exc_info=None):
                     origin_allowed = origin
                 else:
                     # See http://www.w3.org/TR/cors/#access-control-allow-origin-response-header
-                    origin_allowed = origin if origin in origins else 'null'
+                    origin_allowed = origin if origin in origins else list(origins)[0]
             else:
                 origin_allowed = list(origins)[0]
 

st2common/st2common/transport/connection_retry_wrapper.py

@@ -14,6 +14,7 @@
 # limitations under the License.
 
 from __future__ import absolute_import
+
 import eventlet
 
 __all__ = ['ConnectionRetryWrapper', 'ClusterRetryContext']
@@ -35,7 +36,14 @@ def __init__(self, cluster_size):
         # No of nodes attempted. Starts at 1 since the
         self._nodes_attempted = 1
 
-    def test_should_stop(self):
+    def test_should_stop(self, e=None):
+        # Special workaround for "(504) CHANNEL_ERROR - second 'channel.open' seen" which happens
+        # during tests on Travis and block and slown down the tests
+        # NOTE: This error is not fatal during tests and we can simply switch to a next connection
+        # without sleeping.
+        if "second 'channel.open' seen" in str(e):
+            return False, -1
+
         should_stop = True
         if self._nodes_attempted > self.cluster_size * self.cluster_retry:
             return should_stop, -1
@@ -118,7 +126,7 @@ def run(self, connection, wrapped_callback):
                 should_stop = True
             except connection.connection_errors + connection.channel_errors as e:
                 self._logger.exception('RabbitMQ connection or channel error: %s.' % (str(e)))
-                should_stop, wait = self._retry_context.test_should_stop()
+                should_stop, wait = self._retry_context.test_should_stop(e)
                 # reset channel to None to avoid any channel closing errors. At this point
                 # in case of an exception there should be no channel but that is better to
                 # guarantee.
@@ -127,7 +135,10 @@ def run(self, connection, wrapped_callback):
                 # be notified so raise.
                 if should_stop:
                     raise
+
                 # -1, 0 and 1+ are handled properly by eventlet.sleep
+                self._logger.debug('Received RabbitMQ server error, sleeping for %s seconds '
+                                   'before retrying: %s' % (wait, str(e)))
                 eventlet.sleep(wait)
 
                 connection.close()

st2common/tests/unit/test_connection_retry_wrapper.py

@@ -36,6 +36,19 @@ def test_single_node_cluster_retry(self):
         self.assertTrue(should_stop, 'Done trying.')
         self.assertEqual(wait, -1)
 
+    def test_should_stop_second_channel_open_error_should_be_non_fatal(self):
+        retry_context = ClusterRetryContext(cluster_size=1)
+
+        e = Exception("(504) CHANNEL_ERROR - second 'channel.open' seen")
+        should_stop, wait = retry_context.test_should_stop(e=e)
+        self.assertFalse(should_stop)
+        self.assertEqual(wait, -1)
+
+        e = Exception("CHANNEL_ERROR - second 'channel.open' seen")
+        should_stop, wait = retry_context.test_should_stop(e=e)
+        self.assertFalse(should_stop)
+        self.assertEqual(wait, -1)
+
     def test_multiple_node_cluster_retry(self):
         cluster_size = 3
         last_index = cluster_size * 2