Commit 1ac14e9e authored by Rob Watson's avatar Rob Watson
Browse files

Add HTTPS-only pages

Closes #28857
parent 2e0516c21d44
...@@ -21,4 +21,26 @@ def destroy ...@@ -21,4 +21,26 @@ def destroy
end end
end end
end end
def update
result = Projects::UpdateService.new(@project, current_user, project_params).execute
respond_to do |format|
format.html do
if result[:status] == :success
flash[:notice] = 'Your changes have been saved'
else
flash[:alert] = 'Something went wrong on our end'
end
redirect_to project_pages_path(@project)
end
end
end
private
def project_params
params.require(:project).permit(:pages_https_only)
end
end end
...@@ -531,4 +531,22 @@ def find_file_path ...@@ -531,4 +531,22 @@ def find_file_path
def can_show_last_commit_in_list?(project) def can_show_last_commit_in_list?(project)
can?(current_user, :read_cross_project) && project.commit can?(current_user, :read_cross_project) && project.commit
end end
def pages_https_only_disabled?
!@project.pages_domains.all?(&:https?)
end
def pages_https_only_title
return unless pages_https_only_disabled?
"You must enable HTTPS for all your domains first"
end
def pages_https_only_label_class
if pages_https_only_disabled?
"list-label disabled"
else
"list-label"
end
end
end end
...@@ -6,8 +6,10 @@ class PagesDomain < ActiveRecord::Base ...@@ -6,8 +6,10 @@ class PagesDomain < ActiveRecord::Base
validates :domain, hostname: { allow_numeric_hostname: true } validates :domain, hostname: { allow_numeric_hostname: true }
validates :domain, uniqueness: { case_sensitive: false } validates :domain, uniqueness: { case_sensitive: false }
validates :certificate, certificate: true, allow_nil: true, allow_blank: true validates :certificate, presence: { message: 'must be present if HTTPS-only is enabled' }, if: ->(domain) { domain.project&.pages_https_only? }
validates :key, certificate_key: true, allow_nil: true, allow_blank: true validates :certificate, certificate: true, if: ->(domain) { domain.certificate.present? }
validates :key, presence: { message: 'must be present if HTTPS-only is enabled' }, if: ->(domain) { domain.project&.pages_https_only? }
validates :key, certificate_key: true, if: ->(domain) { domain.key.present? }
validates :verification_code, presence: true, allow_blank: false validates :verification_code, presence: true, allow_blank: false
validate :validate_pages_domain validate :validate_pages_domain
...@@ -46,6 +48,10 @@ def enabled? ...@@ -46,6 +48,10 @@ def enabled?
!Gitlab::CurrentSettings.pages_domain_verification_enabled? || enabled_until.present? !Gitlab::CurrentSettings.pages_domain_verification_enabled? || enabled_until.present?
end end
def https?
certificate.present?
end
def to_param def to_param
domain domain
end end
......
...@@ -267,6 +267,7 @@ class Project < ActiveRecord::Base ...@@ -267,6 +267,7 @@ class Project < ActiveRecord::Base
validate :visibility_level_allowed_by_group validate :visibility_level_allowed_by_group
validate :visibility_level_allowed_as_fork validate :visibility_level_allowed_as_fork
validate :check_wiki_path_conflict validate :check_wiki_path_conflict
validate :validate_pages_https_only, if: -> { changes.has_key?(:pages_https_only) }
validates :repository_storage, validates :repository_storage,
presence: true, presence: true,
inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } } inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } }
...@@ -737,6 +738,26 @@ def check_wiki_path_conflict ...@@ -737,6 +738,26 @@ def check_wiki_path_conflict
end end
end end
def pages_https_only
return false unless Gitlab.config.pages.external_https
super
end
def pages_https_only?
return false unless Gitlab.config.pages.external_https
super
end
def validate_pages_https_only
return unless pages_https_only?
unless pages_domains.all?(&:https?)
errors.add(:pages_https_only, "cannot be enabled unless all domains have TLS certificates")
end
end
def to_param def to_param
if persisted? && errors.include?(:path) if persisted? && errors.include?(:path)
path_was path_was
......
...@@ -18,7 +18,8 @@ def execute ...@@ -18,7 +18,8 @@ def execute
def pages_config def pages_config
{ {
domains: pages_domains_config domains: pages_domains_config,
https_only: project.pages_https_only?
} }
end end
...@@ -27,7 +28,8 @@ def pages_domains_config ...@@ -27,7 +28,8 @@ def pages_domains_config
{ {
domain: domain.domain, domain: domain.domain,
certificate: domain.certificate, certificate: domain.certificate,
key: domain.key key: domain.key,
https_only: project.pages_https_only? && domain.https?
} }
end end
end end
......
...@@ -24,6 +24,8 @@ def execute ...@@ -24,6 +24,8 @@ def execute
system_hook_service.execute_hooks_for(project, :update) system_hook_service.execute_hooks_for(project, :update)
end end
update_pages_config if changing_pages_https_only?
success success
else else
model_errors = project.errors.full_messages.to_sentence model_errors = project.errors.full_messages.to_sentence
...@@ -67,5 +69,13 @@ def ensure_wiki_exists ...@@ -67,5 +69,13 @@ def ensure_wiki_exists
log_error("Could not create wiki for #{project.full_name}") log_error("Could not create wiki for #{project.full_name}")
Gitlab::Metrics.counter(:wiki_can_not_be_created_total, 'Counts the times we failed to create a wiki') Gitlab::Metrics.counter(:wiki_can_not_be_created_total, 'Counts the times we failed to create a wiki')
end end
def update_pages_config
Projects::UpdatePagesConfigurationService.new(project).execute
end
def changing_pages_https_only?
project.previous_changes.include?(:pages_https_only)
end
end end
end end
...@@ -16,8 +16,6 @@ def validate_each(record, attribute, value) ...@@ -16,8 +16,6 @@ def validate_each(record, attribute, value)
private private
def valid_certificate_pem?(value) def valid_certificate_pem?(value)
return false unless value
OpenSSL::X509::Certificate.new(value).present? OpenSSL::X509::Certificate.new(value).present?
rescue OpenSSL::X509::CertificateError rescue OpenSSL::X509::CertificateError
false false
......
= form_for @project, url: namespace_project_pages_path(@project.namespace.becomes(Namespace), @project), html: { class: 'inline', title: pages_https_only_title } do |f|
= f.check_box :pages_https_only, class: 'pull-left', disabled: pages_https_only_disabled?
.prepend-left-20
= f.label :pages_https_only, class: pages_https_only_label_class do
%strong Force domains with SSL certificates to use HTTPS
- unless pages_https_only_disabled?
.prepend-top-10
= f.submit 'Save', class: 'btn btn-success'
...@@ -13,6 +13,9 @@ ...@@ -13,6 +13,9 @@
Combined with the power of GitLab CI and the help of GitLab Runner Combined with the power of GitLab CI and the help of GitLab Runner
you can deploy static pages for your individual projects, your user or your group. you can deploy static pages for your individual projects, your user or your group.
- if Gitlab.config.pages.external_https
= render 'https_only'
%hr.clearfix %hr.clearfix
= render 'access' = render 'access'
......
---
title: Add HTTPS-only pages
merge_request: 16273
author: rfwatson
type: added
...@@ -52,7 +52,7 @@ ...@@ -52,7 +52,7 @@
end end
end end
resource :pages, only: [:show, :destroy] do resource :pages, only: [:show, :update, :destroy] do
resources :domains, except: :index, controller: 'pages_domains', constraints: { id: %r{[^/]+} } do resources :domains, except: :index, controller: 'pages_domains', constraints: { id: %r{[^/]+} } do
member do member do
post :verify post :verify
......
class AddPagesHttpsOnlyToProjects < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :projects, :pages_https_only, :boolean
end
end
class ChangeDefaultValueForPagesHttpsOnly < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
change_column_default :projects, :pages_https_only, true
end
def down
change_column_default :projects, :pages_https_only, nil
end
end
...@@ -1513,6 +1513,7 @@ ...@@ -1513,6 +1513,7 @@
t.boolean "merge_requests_ff_only_enabled", default: false t.boolean "merge_requests_ff_only_enabled", default: false
t.boolean "merge_requests_rebase_enabled", default: false, null: false t.boolean "merge_requests_rebase_enabled", default: false, null: false
t.integer "jobs_cache_index" t.integer "jobs_cache_index"
t.boolean "pages_https_only", default: true
end end
add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree
......
...@@ -65,4 +65,41 @@ ...@@ -65,4 +65,41 @@
end end
end end
end end
describe 'PATCH update' do
let(:request_params) do
{
namespace_id: project.namespace,
project_id: project,
project: { pages_https_only: false }
}
end
let(:update_service) { double(execute: { status: :success }) }
before do
allow(Projects::UpdateService).to receive(:new) { update_service }
end
it 'returns 302 status' do
patch :update, request_params
expect(response).to have_gitlab_http_status(:found)
end
it 'redirects back to the pages settings' do
patch :update, request_params
expect(response).to redirect_to(project_pages_path(project))
end
it 'calls the update service' do
expect(Projects::UpdateService)
.to receive(:new)
.with(project, user, request_params[:project])
.and_return(update_service)
patch :update, request_params
end
end
end end
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
end end
let(:pages_domain_params) do let(:pages_domain_params) do
build(:pages_domain, :with_certificate, :with_key, domain: 'my.otherdomain.com').slice(:key, :certificate, :domain) build(:pages_domain, domain: 'my.otherdomain.com').slice(:key, :certificate, :domain)
end end
before do before do
...@@ -68,7 +68,7 @@ ...@@ -68,7 +68,7 @@
end end
let(:pages_domain_params) do let(:pages_domain_params) do
attributes_for(:pages_domain, :with_certificate, :with_key).slice(:key, :certificate) attributes_for(:pages_domain).slice(:key, :certificate)
end end
let(:params) do let(:params) do
......
...@@ -4,25 +4,7 @@ ...@@ -4,25 +4,7 @@
verified_at { Time.now } verified_at { Time.now }
enabled_until { 1.week.from_now } enabled_until { 1.week.from_now }
trait :disabled do certificate '-----BEGIN CERTIFICATE-----
verified_at nil
enabled_until nil
end
trait :unverified do
verified_at nil
end
trait :reverify do
enabled_until { 1.hour.from_now }
end
trait :expired do
enabled_until { 1.hour.ago }
end
trait :with_certificate do
certificate '-----BEGIN CERTIFICATE-----
MIICGzCCAYSgAwIBAgIBATANBgkqhkiG9w0BAQUFADAbMRkwFwYDVQQDExB0ZXN0 MIICGzCCAYSgAwIBAgIBATANBgkqhkiG9w0BAQUFADAbMRkwFwYDVQQDExB0ZXN0
LWNlcnRpZmljYXRlMB4XDTE2MDIxMjE0MzIwMFoXDTIwMDQxMjE0MzIwMFowGzEZ LWNlcnRpZmljYXRlMB4XDTE2MDIxMjE0MzIwMFoXDTIwMDQxMjE0MzIwMFowGzEZ
MBcGA1UEAxMQdGVzdC1jZXJ0aWZpY2F0ZTCBnzANBgkqhkiG9w0BAQEFAAOBjQAw MBcGA1UEAxMQdGVzdC1jZXJ0aWZpY2F0ZTCBnzANBgkqhkiG9w0BAQEFAAOBjQAw
...@@ -36,10 +18,8 @@ ...@@ -36,10 +18,8 @@
5A4K65Nb7Oh1AdQieTBHNXXCdyFsva9/ScfQGEl7p55a52jOPs0StPd7g64uvjlg 5A4K65Nb7Oh1AdQieTBHNXXCdyFsva9/ScfQGEl7p55a52jOPs0StPd7g64uvjlg
YHi2yesCrOvVXt+lgPTd YHi2yesCrOvVXt+lgPTd
-----END CERTIFICATE-----' -----END CERTIFICATE-----'
end
trait :with_key do key '-----BEGIN PRIVATE KEY-----
key '-----BEGIN PRIVATE KEY-----
MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKS+CfS9GcRSdYSN MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKS+CfS9GcRSdYSN
SzyH5QJQBr5umRL6E+KilOV39iYFO/9oHjUdapTRWkrwnNPCp7qaeck4Jr8iv14t SzyH5QJQBr5umRL6E+KilOV39iYFO/9oHjUdapTRWkrwnNPCp7qaeck4Jr8iv14t
PVNDfNr76eGb6/3YknOAP0QOjLWunoC8kjU+N/JHU52NrUeX3qEy8EKV9LeCDJcB PVNDfNr76eGb6/3YknOAP0QOjLWunoC8kjU+N/JHU52NrUeX3qEy8EKV9LeCDJcB
...@@ -55,6 +35,30 @@ ...@@ -55,6 +35,30 @@
63BDJEwvOb2IaP8lDDxNsXx9XJNVvQbv5n15vNsLHbjslHfAhAbxnLQ1fLhUPqSi 63BDJEwvOb2IaP8lDDxNsXx9XJNVvQbv5n15vNsLHbjslHfAhAbxnLQ1fLhUPqSi
nNp/xedE1YxutQ== nNp/xedE1YxutQ==
-----END PRIVATE KEY-----' -----END PRIVATE KEY-----'
trait :disabled do
verified_at nil
enabled_until nil
end
trait :unverified do
verified_at nil
end
trait :reverify do
enabled_until { 1.hour.from_now }
end
trait :expired do
enabled_until { 1.hour.ago }
end
trait :without_certificate do
certificate nil
end
trait :without_key do
key nil
end end
trait :with_missing_chain do trait :with_missing_chain do
......
...@@ -40,11 +40,6 @@ ...@@ -40,11 +40,6 @@
end end
context 'when support for external domains is disabled' do context 'when support for external domains is disabled' do
before do
allow(Gitlab.config.pages).to receive(:external_http).and_return(nil)
allow(Gitlab.config.pages).to receive(:external_https).and_return(nil)
end
it 'renders message that support is disabled' do it 'renders message that support is disabled' do
visit project_pages_path(project) visit project_pages_path(project)
...@@ -52,7 +47,9 @@ ...@@ -52,7 +47,9 @@
end end
end end
context 'when pages are exposed on external HTTP address' do context 'when pages are exposed on external HTTP address', :http_pages_enabled do
given(:project) { create(:project, pages_https_only: false) }
shared_examples 'adds new domain' do shared_examples 'adds new domain' do
it 'adds new domain' do it 'adds new domain' do
visit new_project_pages_domain_path(project) visit new_project_pages_domain_path(project)
...@@ -64,11 +61,6 @@ ...@@ -64,11 +61,6 @@
end end
end end
before do
allow(Gitlab.config.pages).to receive(:external_http).and_return(['1.1.1.1:80'])
allow(Gitlab.config.pages).to receive(:external_https).and_return(nil)
end
it 'allows to add new domain' do it 'allows to add new domain' do
visit project_pages_path(project) visit project_pages_path(project)
...@@ -80,13 +72,13 @@ ...@@ -80,13 +72,13 @@
context 'when project in group namespace' do context 'when project in group namespace' do
it_behaves_like 'adds new domain' do it_behaves_like 'adds new domain' do
let(:group) { create :group } let(:group) { create :group }
let(:project) { create :project, namespace: group } let(:project) { create(:project, namespace: group, pages_https_only: false) }
end end
end end
context 'when pages domain is added' do context 'when pages domain is added' do
before do before do
project.pages_domains.create!(domain: 'my.test.domain.com') create(:pages_domain, project: project, domain: 'my.test.domain.com')
visit new_project_pages_domain_path(project) visit new_project_pages_domain_path(project)
end end
...@@ -104,7 +96,7 @@ ...@@ -104,7 +96,7 @@
end end
end end
context 'when pages are exposed on external HTTPS address' do context 'when pages are exposed on external HTTPS address', :https_pages_enabled do
let(:certificate_pem) do let(:certificate_pem) do
<<~PEM <<~PEM
-----BEGIN CERTIFICATE----- -----BEGIN CERTIFICATE-----
...@@ -145,11 +137,6 @@ ...@@ -145,11 +137,6 @@
KEY KEY
end end
before do
allow(Gitlab.config.pages).to receive(:external_http).and_return(['1.1.1.1:80'])
allow(Gitlab.config.pages).to receive(:external_https).and_return(['1.1.1.1:443'])
end
it 'adds new domain with certificate' do it 'adds new domain with certificate' do
visit new_project_pages_domain_path(project) visit new_project_pages_domain_path(project)
...@@ -163,7 +150,7 @@ ...@@ -163,7 +150,7 @@
describe 'updating the certificate for an existing domain' do describe 'updating the certificate for an existing domain' do
let!(:domain) do let!(:domain) do
create(:pages_domain, :with_key, :with_certificate, project: project) create(:pages_domain, project: project)
end end
it 'allows the certificate to be updated' do it 'allows the certificate to be updated' do
...@@ -237,6 +224,69 @@ ...@@ -237,6 +224,69 @@
it_behaves_like 'no pages deployed' it_behaves_like 'no pages deployed'
end end
describe 'HTTPS settings', :js, :https_pages_enabled do
background do
project.namespace.update(owner: user)
allow_any_instance_of(Project).to receive(:pages_deployed?) { true }
end
scenario 'tries to change the setting' do
visit project_pages_path(project)
expect(page).to have_content("Force domains with SSL certificates to use HTTPS")
uncheck :project_pages_https_only
click_button 'Save'
expect(page).to have_text('Your changes have been saved')
expect(page).not_to have_checked_field('project_pages_https_only')
end
context 'setting could not be updated' do
before do
allow_any_instance_of(Projects::UpdateService)
.to receive(:execute)
.and_return(status: :error)
end
scenario 'tries to change the setting' do
visit project_pages_path(project)
uncheck :project_pages_https_only
click_button 'Save'
expect(page).to have_text('Something went wrong on our end')
end
end