diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 33c490a205..9d3fe83d17 100644 --- a/CHANGELOG.rst +++ b/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 ------------------------- diff --git a/Makefile b/Makefile index 732dce0eb0..4cf5e4dd62 100644 --- a/Makefile +++ b/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: diff --git a/st2api/tests/unit/controllers/v1/test_base.py b/st2api/tests/unit/controllers/v1/test_base.py index 2a753f22ea..69845c76be 100644 --- a/st2api/tests/unit/controllers/v1/test_base.py +++ b/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: diff --git a/st2common/st2common/middleware/cors.py b/st2common/st2common/middleware/cors.py index 5781b1a6e7..8cb407b52c 100644 --- a/st2common/st2common/middleware/cors.py +++ b/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] diff --git a/st2common/st2common/transport/connection_retry_wrapper.py b/st2common/st2common/transport/connection_retry_wrapper.py index a471ab236f..ef817a0705 100644 --- a/st2common/st2common/transport/connection_retry_wrapper.py +++ b/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() diff --git a/st2common/tests/unit/test_connection_retry_wrapper.py b/st2common/tests/unit/test_connection_retry_wrapper.py index 80c998eab1..97ad1fc035 100644 --- a/st2common/tests/unit/test_connection_retry_wrapper.py +++ b/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