From 289d5bb66ba404244e567c31027c5ed6928b02d8 Mon Sep 17 00:00:00 2001 From: Nobuto Murata Date: Sat, 26 Aug 2017 18:10:22 +0700 Subject: [PATCH] Rely on HTTP_HOST sent by clients for redirection The dashboard may have multiple networks and IP addresses. We never be able to determine where to redirect reliably. Also, redirecting an access from internal network to a public IP address may not be what users want. Instead, use HTTP_HOST sent by the client and let the client's browser reveal SSL related errors if any. Change-Id: I9f4c734a61d3ab07f3f7c9a1a073eede73ae4651 Closes-Bug: #1710930 Closes-Bug: #1713198 --- hooks/horizon_contexts.py | 8 +++----- hooks/horizon_utils.py | 1 + templates/default | 7 ++++--- unit_tests/test_horizon_contexts.py | 19 +++++++++++++------ unit_tests/test_horizon_utils.py | 3 ++- 5 files changed, 23 insertions(+), 15 deletions(-) diff --git a/hooks/horizon_contexts.py b/hooks/horizon_contexts.py index ff3ec29a..a4c75c7d 100644 --- a/hooks/horizon_contexts.py +++ b/hooks/horizon_contexts.py @@ -31,9 +31,6 @@ from charmhelpers.contrib.openstack.context import ( HAProxyContext, context_complete ) -from charmhelpers.contrib.openstack.ip import ( - resolve_address, -) from charmhelpers.contrib.openstack.utils import ( git_default_repos, git_pip_venv_dir, @@ -211,13 +208,14 @@ class ApacheContext(OSContextGenerator): ''' Grab cert and key from configuraton for SSL config ''' ctxt = { 'http_port': 70, - 'https_port': 433 + 'https_port': 433, + 'enforce_ssl': False } if config('enforce-ssl'): # NOTE(dosaboy): if ssl is not configured we shouldn't allow this if all(get_cert()): - ctxt['ssl_addr'] = resolve_address() + ctxt['enforce_ssl'] = True else: log("Enforce ssl redirect requested but ssl not configured - " "skipping redirect", level=WARNING) diff --git a/hooks/horizon_utils.py b/hooks/horizon_utils.py index efc1cda3..e94ac8d3 100644 --- a/hooks/horizon_utils.py +++ b/hooks/horizon_utils.py @@ -263,6 +263,7 @@ def enable_ssl(): ''' Enable SSL support in local apache2 instance ''' subprocess.call(['a2ensite', 'default-ssl']) subprocess.call(['a2enmod', 'ssl']) + subprocess.call(['a2enmod', 'rewrite']) def determine_packages(): diff --git a/templates/default b/templates/default index b19755f1..1ee4966f 100644 --- a/templates/default +++ b/templates/default @@ -1,8 +1,9 @@ - {% if ssl_addr -%} - RedirectPermanent / https://{{ ssl_addr }}:443/ - {%- endif %} +{% if enforce_ssl %} + RewriteEngine On + RewriteRule ^(.*)$ https://%{HTTP_HOST}%{REQUEST_URI} [L,R=301] +{% endif %} ServerAdmin webmaster@localhost DocumentRoot /var/www diff --git a/unit_tests/test_horizon_contexts.py b/unit_tests/test_horizon_contexts.py index ab4bb60d..cbcb0d67 100644 --- a/unit_tests/test_horizon_contexts.py +++ b/unit_tests/test_horizon_contexts.py @@ -32,7 +32,6 @@ TO_PATCH = [ 'local_unit', 'unit_get', 'pwgen', - 'resolve_address', ] @@ -63,14 +62,22 @@ class TestHorizonContexts(CharmTestCase): def test_Apachecontext(self): self.assertEqual(horizon_contexts.ApacheContext()(), - {'http_port': 70, 'https_port': 433}) + {'http_port': 70, 'https_port': 433, + 'enforce_ssl': False}) def test_Apachecontext_enforce_ssl(self): self.test_config.set('enforce-ssl', True) - self.resolve_address.return_value = 'horizon.example.stack' - self.assertEqual(horizon_contexts.ApacheContext()(), - {'http_port': 70, 'https_port': 433, - 'ssl_addr': 'horizon.example.stack'}) + self.get_cert.return_value = ('cert', 'key') + self.assertEquals(horizon_contexts.ApacheContext()(), + {'http_port': 70, 'https_port': 433, + 'enforce_ssl': True}) + + def test_Apachecontext_enforce_ssl_no_cert(self): + self.test_config.set('enforce-ssl', True) + self.get_cert.return_value = (None, 'key') + self.assertEquals(horizon_contexts.ApacheContext()(), + {'http_port': 70, 'https_port': 433, + 'enforce_ssl': False}) @patch.object(horizon_contexts, 'get_ca_cert', lambda: None) @patch('os.chmod') diff --git a/unit_tests/test_horizon_utils.py b/unit_tests/test_horizon_utils.py index 891a4041..d40ca0eb 100644 --- a/unit_tests/test_horizon_utils.py +++ b/unit_tests/test_horizon_utils.py @@ -80,7 +80,8 @@ class TestHorizohorizon_utils(CharmTestCase): horizon_utils.enable_ssl() _call.assert_has_calls([ call(['a2ensite', 'default-ssl']), - call(['a2enmod', 'ssl']) + call(['a2enmod', 'ssl']), + call(['a2enmod', 'rewrite']) ]) def test_restart_map(self):