From 61b5509390be98dd9ead42fee930442f179b5afd Mon Sep 17 00:00:00 2001 From: Sascha Doering Date: Mon, 7 Oct 2019 16:01:22 +0200 Subject: [PATCH] Move ssl_redirect into a location Fixes GH-1347 --- spec/acceptance/nginx_server_spec.rb | 85 ++++++++++++++++++++++++++++ spec/defines/resource_server_spec.rb | 2 +- templates/server/server_header.erb | 9 ++- 3 files changed, 92 insertions(+), 4 deletions(-) diff --git a/spec/acceptance/nginx_server_spec.rb b/spec/acceptance/nginx_server_spec.rb index 8d93d5583..19b30cd85 100755 --- a/spec/acceptance/nginx_server_spec.rb +++ b/spec/acceptance/nginx_server_spec.rb @@ -159,4 +159,89 @@ class { 'nginx': } end end end + + context 'should run successfully with ssl_redirect' do + it 'configures a nginx SSL server' do + pp = " + class { 'nginx': } + nginx::resource::server { 'www.puppetlabs.com': + ensure => present, + ssl => true, + ssl_cert => '/etc/pki/tls/certs/blah.cert', + ssl_key => '/etc/pki/tls/private/blah.key', + ssl_redirect => true, + www_root => '/var/www/www.puppetlabs.com', + } + nginx::resource::location { 'letsencrypt': + location => '^~ /.well-known/acme-challenge', + www_root => '/var/www/letsencrypt', + index_files => [], + ssl => false, + server => ['www.puppetlabs.com'], + } + host { 'www.puppetlabs.com': ip => '127.0.0.1', } + file { ['/var/www','/var/www/www.puppetlabs.com','/var/www/letsencrypt','/var/www/letsencrypt/.well-known','/var/www/letsencrypt/.well-known/acme-challenge']: ensure => directory } + file { '/var/www/www.puppetlabs.com/index.html': ensure => file, content => 'Hello from www\n', } + file { '/var/www/letsencrypt/.well-known/acme-challenge/fb9bd98604be3d0c7d589fcc7561cb41': ensure => file, content => 'LetsEncrypt\n', } + " + + apply_manifest(pp, catch_failures: true) + end + + describe file('/etc/nginx/sites-available/www.puppetlabs.com.conf') do + it { is_expected.to be_file } + it { is_expected.to contain 'return 301 https://$host$request_uri;' } + end + + describe service('nginx') do + it { is_expected.to be_running } + end + + describe port(80) do + it { is_expected.to be_listening } + end + + describe port(443) do + it { is_expected.to be_listening } + end + + it 'answers to http://www.puppetlabs.com with redirect to HTTPS' do + shell('/usr/bin/curl -I http://www.puppetlabs.com:80') do |r| + expect(r.stdout).to contain('301 Moved Permanently') + end + end + + it 'answers to http://www.puppetlabs.com with redirect to HTTPS' do + shell('/usr/bin/curl -I http://www.puppetlabs.com:80') do |r| + expect(r.stdout).to contain('Location: https://www.puppetlabs.com') + end + end + + it 'answers to http://www.puppetlabs.com without error' do + shell('/usr/bin/curl --fail http://www.puppetlabs.com:80') do |r| + expect(r.exit_code).to eq(0) + end + end + + it 'answers to https://www.puppetlabs.com with "Hello from www"' do + # use --insecure because it's a self-signed cert + shell('/usr/bin/curl --insecure https://www.puppetlabs.com:443') do |r| + expect(r.stdout).to eq("Hello from www\n") + end + end + + it 'answers to http://www.puppetlabs.com/.well-known/acme-challenge/fb9bd98604be3d0c7d589fcc7561cb41 with "LetsEncrypt"' do + # use --insecure because it's a self-signed cert + shell('/usr/bin/curl http://www.puppetlabs.com:80/.well-known/acme-challenge/fb9bd98604be3d0c7d589fcc7561cb41') do |r| + expect(r.stdout).to eq("LetsEncrypt\n") + end + end + + it 'answers to https://www.puppetlabs.com/.well-known/acme-challenge/fb9bd98604be3d0c7d589fcc7561cb41 with "LetsEncrypt"' do + # use --insecure because it's a self-signed cert + shell('/usr/bin/curl --insecure https://www.puppetlabs.com:443/.well-known/acme-challenge/fb9bd98604be3d0c7d589fcc7561cb41') do |r| + expect(r.stdout).to contain('404 Not Found') + end + end + end end diff --git a/spec/defines/resource_server_spec.rb b/spec/defines/resource_server_spec.rb index efde6b3b7..bb42ad5e0 100644 --- a/spec/defines/resource_server_spec.rb +++ b/spec/defines/resource_server_spec.rb @@ -1097,7 +1097,7 @@ let(:params) { { ssl_redirect: true } } it { is_expected.to contain_concat__fragment("#{title}-header").without_content(%r{^\s*index\s+}) } - it { is_expected.to contain_concat__fragment("#{title}-header").without_content(%r{^\s*location\s+}) } + it { is_expected.to contain_concat__fragment("#{title}-header").with_content(%r{ return 301 https://\$host\$request_uri;}) } end context 'ssl_redirect with alternate port' do diff --git a/templates/server/server_header.erb b/templates/server/server_header.erb index 919515140..6f44b7d88 100644 --- a/templates/server/server_header.erb +++ b/templates/server/server_header.erb @@ -134,9 +134,6 @@ server { <% if @maintenance -%> <%= @maintenance_value %>; <% end -%> -<% if @ssl_redirect -%> - return 301 https://$host<% if @_ssl_redirect_port.to_i != 443 %>:<%= @_ssl_redirect_port %><% end %>$request_uri; -<% end -%> <% if @index_files and @index_files.count > 0 and not @ssl_only -%> index <% Array(@index_files).each do |i| %> <%= i %><% end %>; <% end -%> @@ -180,3 +177,9 @@ server { error_page <%= key %> <%= @error_pages[key] %>; <%- end -%> <% end -%> +<% if @ssl_redirect -%> + + location / { + return 301 https://$host<% if @_ssl_redirect_port.to_i != 443 %>:<%= @_ssl_redirect_port %><% end %>$request_uri; + } +<% end -%>