summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Thomas <nick@gitlab.com>2018-03-23 15:48:28 +0000
committerNick Thomas <nick@gitlab.com>2018-03-23 15:48:28 +0000
commit457a99c612acf59ec9cc4d0793c53465f914e696 (patch)
treeca474ece3e4db339d225d9789298ee78051848eb
parent79dd74db14ffdac34fa2eff0aa26387b29a16cb5 (diff)
parent9d45951fcaeda4f01a2e4be2480d980a3e7cd37e (diff)
downloadgitlab-ce-457a99c612acf59ec9cc4d0793c53465f914e696.tar.gz
Merge branch 'pages_force_https' into 'master'
HTTPS only Pages Closes #28857 See merge request gitlab-org/gitlab-ce!16273
-rw-r--r--app/controllers/projects/pages_controller.rb22
-rw-r--r--app/helpers/projects_helper.rb18
-rw-r--r--app/models/pages_domain.rb10
-rw-r--r--app/models/project.rb21
-rw-r--r--app/services/projects/update_pages_configuration_service.rb6
-rw-r--r--app/services/projects/update_service.rb10
-rw-r--r--app/validators/certificate_validator.rb2
-rw-r--r--app/views/projects/pages/_https_only.html.haml10
-rw-r--r--app/views/projects/pages/show.html.haml3
-rw-r--r--changelogs/unreleased/pages_force_https.yml5
-rw-r--r--config/routes/project.rb2
-rw-r--r--db/migrate/20180102220145_add_pages_https_only_to_projects.rb9
-rw-r--r--db/migrate/20180109183319_change_default_value_for_pages_https_only.rb13
-rw-r--r--db/schema.rb1
-rw-r--r--spec/controllers/projects/pages_controller_spec.rb37
-rw-r--r--spec/controllers/projects/pages_domains_controller_spec.rb4
-rw-r--r--spec/factories/pages_domains.rb48
-rw-r--r--spec/features/projects/pages_spec.rb90
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml1
-rw-r--r--spec/models/pages_domain_spec.rb146
-rw-r--r--spec/models/project_spec.rb45
-rw-r--r--spec/requests/api/pages_domains_spec.rb14
-rw-r--r--spec/services/projects/update_service_spec.rb21
-rw-r--r--spec/spec_helper.rb16
24 files changed, 447 insertions, 107 deletions
diff --git a/app/controllers/projects/pages_controller.rb b/app/controllers/projects/pages_controller.rb
index d421b1a8eb5..cae6e2c40b8 100644
--- a/app/controllers/projects/pages_controller.rb
+++ b/app/controllers/projects/pages_controller.rb
@@ -21,4 +21,26 @@ class Projects::PagesController < Projects::ApplicationController
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
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index da9fe734f1c..15f48e43a28 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -531,4 +531,22 @@ module ProjectsHelper
def can_show_last_commit_in_list?(project)
can?(current_user, :read_cross_project) && project.commit
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
diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb
index 588bd50ed77..2e478a24778 100644
--- a/app/models/pages_domain.rb
+++ b/app/models/pages_domain.rb
@@ -6,8 +6,10 @@ class PagesDomain < ActiveRecord::Base
validates :domain, hostname: { allow_numeric_hostname: true }
validates :domain, uniqueness: { case_sensitive: false }
- validates :certificate, certificate: true, allow_nil: true, allow_blank: true
- validates :key, certificate_key: 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 :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
validate :validate_pages_domain
@@ -46,6 +48,10 @@ class PagesDomain < ActiveRecord::Base
!Gitlab::CurrentSettings.pages_domain_verification_enabled? || enabled_until.present?
end
+ def https?
+ certificate.present?
+ end
+
def to_param
domain
end
diff --git a/app/models/project.rb b/app/models/project.rb
index ed5f8b00ba2..6a420663644 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -267,6 +267,7 @@ class Project < ActiveRecord::Base
validate :visibility_level_allowed_by_group
validate :visibility_level_allowed_as_fork
validate :check_wiki_path_conflict
+ validate :validate_pages_https_only, if: -> { changes.has_key?(:pages_https_only) }
validates :repository_storage,
presence: true,
inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } }
@@ -737,6 +738,26 @@ class Project < ActiveRecord::Base
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
if persisted? && errors.include?(:path)
path_was
diff --git a/app/services/projects/update_pages_configuration_service.rb b/app/services/projects/update_pages_configuration_service.rb
index 52ff64cc938..25017c5cbe3 100644
--- a/app/services/projects/update_pages_configuration_service.rb
+++ b/app/services/projects/update_pages_configuration_service.rb
@@ -18,7 +18,8 @@ module Projects
def pages_config
{
- domains: pages_domains_config
+ domains: pages_domains_config,
+ https_only: project.pages_https_only?
}
end
@@ -27,7 +28,8 @@ module Projects
{
domain: domain.domain,
certificate: domain.certificate,
- key: domain.key
+ key: domain.key,
+ https_only: project.pages_https_only? && domain.https?
}
end
end
diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb
index 5f2615a2c01..679f4a9cb62 100644
--- a/app/services/projects/update_service.rb
+++ b/app/services/projects/update_service.rb
@@ -24,6 +24,8 @@ module Projects
system_hook_service.execute_hooks_for(project, :update)
end
+ update_pages_config if changing_pages_https_only?
+
success
else
model_errors = project.errors.full_messages.to_sentence
@@ -67,5 +69,13 @@ module Projects
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')
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
diff --git a/app/validators/certificate_validator.rb b/app/validators/certificate_validator.rb
index 5239e70a326..b0c9a1b92a4 100644
--- a/app/validators/certificate_validator.rb
+++ b/app/validators/certificate_validator.rb
@@ -16,8 +16,6 @@ class CertificateValidator < ActiveModel::EachValidator
private
def valid_certificate_pem?(value)
- return false unless value
-
OpenSSL::X509::Certificate.new(value).present?
rescue OpenSSL::X509::CertificateError
false
diff --git a/app/views/projects/pages/_https_only.html.haml b/app/views/projects/pages/_https_only.html.haml
new file mode 100644
index 00000000000..6a3ffce949f
--- /dev/null
+++ b/app/views/projects/pages/_https_only.html.haml
@@ -0,0 +1,10 @@
+= 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'
diff --git a/app/views/projects/pages/show.html.haml b/app/views/projects/pages/show.html.haml
index 04e647c0dc6..f17d9d24db6 100644
--- a/app/views/projects/pages/show.html.haml
+++ b/app/views/projects/pages/show.html.haml
@@ -13,6 +13,9 @@
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.
+- if Gitlab.config.pages.external_https
+ = render 'https_only'
+
%hr.clearfix
= render 'access'
diff --git a/changelogs/unreleased/pages_force_https.yml b/changelogs/unreleased/pages_force_https.yml
new file mode 100644
index 00000000000..da7e29087f3
--- /dev/null
+++ b/changelogs/unreleased/pages_force_https.yml
@@ -0,0 +1,5 @@
+---
+title: Add HTTPS-only pages
+merge_request: 16273
+author: rfwatson
+type: added
diff --git a/config/routes/project.rb b/config/routes/project.rb
index c803737d40b..f50b9aded8d 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -52,7 +52,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
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
member do
post :verify
diff --git a/db/migrate/20180102220145_add_pages_https_only_to_projects.rb b/db/migrate/20180102220145_add_pages_https_only_to_projects.rb
new file mode 100644
index 00000000000..ef6bc6896c0
--- /dev/null
+++ b/db/migrate/20180102220145_add_pages_https_only_to_projects.rb
@@ -0,0 +1,9 @@
+class AddPagesHttpsOnlyToProjects < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ def change
+ add_column :projects, :pages_https_only, :boolean
+ end
+end
diff --git a/db/migrate/20180109183319_change_default_value_for_pages_https_only.rb b/db/migrate/20180109183319_change_default_value_for_pages_https_only.rb
new file mode 100644
index 00000000000..c242e1b0d24
--- /dev/null
+++ b/db/migrate/20180109183319_change_default_value_for_pages_https_only.rb
@@ -0,0 +1,13 @@
+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
diff --git a/db/schema.rb b/db/schema.rb
index 56116a2d241..1be0570f85a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1513,6 +1513,7 @@ ActiveRecord::Schema.define(version: 20180320182229) do
t.boolean "merge_requests_ff_only_enabled", default: false
t.boolean "merge_requests_rebase_enabled", default: false, null: false
t.integer "jobs_cache_index"
+ t.boolean "pages_https_only", default: true
end
add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree
diff --git a/spec/controllers/projects/pages_controller_spec.rb b/spec/controllers/projects/pages_controller_spec.rb
index 4705c50de7e..11f54eef531 100644
--- a/spec/controllers/projects/pages_controller_spec.rb
+++ b/spec/controllers/projects/pages_controller_spec.rb
@@ -65,4 +65,41 @@ describe Projects::PagesController do
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
diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb
index 83a3799e883..d4058a5c515 100644
--- a/spec/controllers/projects/pages_domains_controller_spec.rb
+++ b/spec/controllers/projects/pages_domains_controller_spec.rb
@@ -13,7 +13,7 @@ describe Projects::PagesDomainsController do
end
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
before do
@@ -68,7 +68,7 @@ describe Projects::PagesDomainsController do
end
let(:pages_domain_params) do
- attributes_for(:pages_domain, :with_certificate, :with_key).slice(:key, :certificate)
+ attributes_for(:pages_domain).slice(:key, :certificate)
end
let(:params) do
diff --git a/spec/factories/pages_domains.rb b/spec/factories/pages_domains.rb
index 35b44e1c52e..20671da016e 100644
--- a/spec/factories/pages_domains.rb
+++ b/spec/factories/pages_domains.rb
@@ -4,25 +4,7 @@ FactoryBot.define do
verified_at { Time.now }
enabled_until { 1.week.from_now }
- 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 :with_certificate do
- certificate '-----BEGIN CERTIFICATE-----
+ certificate '-----BEGIN CERTIFICATE-----
MIICGzCCAYSgAwIBAgIBATANBgkqhkiG9w0BAQUFADAbMRkwFwYDVQQDExB0ZXN0
LWNlcnRpZmljYXRlMB4XDTE2MDIxMjE0MzIwMFoXDTIwMDQxMjE0MzIwMFowGzEZ
MBcGA1UEAxMQdGVzdC1jZXJ0aWZpY2F0ZTCBnzANBgkqhkiG9w0BAQEFAAOBjQAw
@@ -36,10 +18,8 @@ joZp2JHYvNlTPkRJ/J4TcXxBTJmArcQgTIuNoBtC+0A/SwdK4MfTCUY4vNWNdese
5A4K65Nb7Oh1AdQieTBHNXXCdyFsva9/ScfQGEl7p55a52jOPs0StPd7g64uvjlg
YHi2yesCrOvVXt+lgPTd
-----END CERTIFICATE-----'
- end
- trait :with_key do
- key '-----BEGIN PRIVATE KEY-----
+ key '-----BEGIN PRIVATE KEY-----
MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKS+CfS9GcRSdYSN
SzyH5QJQBr5umRL6E+KilOV39iYFO/9oHjUdapTRWkrwnNPCp7qaeck4Jr8iv14t
PVNDfNr76eGb6/3YknOAP0QOjLWunoC8kjU+N/JHU52NrUeX3qEy8EKV9LeCDJcB
@@ -55,6 +35,30 @@ EPjGlXIT+aW2XiPmK3ZlCDcWIenE+lmtbOpI159Wpk8BGXs/s/xBAkEAlAY3ymgx
63BDJEwvOb2IaP8lDDxNsXx9XJNVvQbv5n15vNsLHbjslHfAhAbxnLQ1fLhUPqSi
nNp/xedE1YxutQ==
-----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
trait :with_missing_chain do
diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb
index 233d2e67b9d..020738ae865 100644
--- a/spec/features/projects/pages_spec.rb
+++ b/spec/features/projects/pages_spec.rb
@@ -40,11 +40,6 @@ feature 'Pages' do
end
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
visit project_pages_path(project)
@@ -52,7 +47,9 @@ feature 'Pages' do
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
it 'adds new domain' do
visit new_project_pages_domain_path(project)
@@ -64,11 +61,6 @@ feature 'Pages' do
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
visit project_pages_path(project)
@@ -80,13 +72,13 @@ feature 'Pages' do
context 'when project in group namespace' do
it_behaves_like 'adds new domain' do
let(:group) { create :group }
- let(:project) { create :project, namespace: group }
+ let(:project) { create(:project, namespace: group, pages_https_only: false) }
end
end
context 'when pages domain is added' 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)
end
@@ -104,7 +96,7 @@ feature 'Pages' do
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
<<~PEM
-----BEGIN CERTIFICATE-----
@@ -145,11 +137,6 @@ feature 'Pages' do
KEY
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
visit new_project_pages_domain_path(project)
@@ -163,7 +150,7 @@ feature 'Pages' do
describe 'updating the certificate for an existing domain' do
let!(:domain) do
- create(:pages_domain, :with_key, :with_certificate, project: project)
+ create(:pages_domain, project: project)
end
it 'allows the certificate to be updated' do
@@ -237,6 +224,69 @@ feature 'Pages' do
it_behaves_like 'no pages deployed'
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
+
+ context 'non-HTTPS domain exists' do
+ given(:project) { create(:project, pages_https_only: false) }
+
+ before do
+ create(:pages_domain, :without_key, :without_certificate, project: project)
+ end
+
+ scenario 'the setting is disabled' do
+ visit project_pages_path(project)
+
+ expect(page).to have_field(:project_pages_https_only, disabled: true)
+ expect(page).not_to have_button('Save')
+ end
+ end
+
+ context 'HTTPS pages are disabled', :https_pages_disabled do
+ scenario 'the setting is unavailable' do
+ visit project_pages_path(project)
+
+ expect(page).not_to have_field(:project_pages_https_only)
+ expect(page).not_to have_content('Force domains with SSL certificates to use HTTPS')
+ expect(page).not_to have_button('Save')
+ end
+ end
+ end
+
describe 'Remove page' do
context 'when user is the owner' do
let(:project) { create :project, :repository }
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 0b938892da5..44e4c6ff94b 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -458,6 +458,7 @@ Project:
- merge_requests_ff_only_enabled
- merge_requests_rebase_enabled
- jobs_cache_index
+- pages_https_only
Author:
- name
ProjectFeature:
diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb
index 95713d8b85b..4b85c5e8720 100644
--- a/spec/models/pages_domain_spec.rb
+++ b/spec/models/pages_domain_spec.rb
@@ -18,24 +18,63 @@ describe PagesDomain do
it { is_expected.to validate_uniqueness_of(:domain).case_insensitive }
end
- {
- 'my.domain.com' => true,
- '123.456.789' => true,
- '0x12345.com' => true,
- '0123123' => true,
- '_foo.com' => false,
- 'reserved.com' => false,
- 'a.reserved.com' => false,
- nil => false
- }.each do |value, validity|
- context "domain #{value.inspect} validity" do
- before do
- allow(Settings.pages).to receive(:host).and_return('reserved.com')
+ describe "hostname" do
+ {
+ 'my.domain.com' => true,
+ '123.456.789' => true,
+ '0x12345.com' => true,
+ '0123123' => true,
+ '_foo.com' => false,
+ 'reserved.com' => false,
+ 'a.reserved.com' => false,
+ nil => false
+ }.each do |value, validity|
+ context "domain #{value.inspect} validity" do
+ before do
+ allow(Settings.pages).to receive(:host).and_return('reserved.com')
+ end
+
+ let(:domain) { value }
+
+ it { expect(pages_domain.valid?).to eq(validity) }
+ end
+ end
+ end
+
+ describe "HTTPS-only" do
+ using RSpec::Parameterized::TableSyntax
+
+ let(:domain) { 'my.domain.com' }
+
+ let(:project) do
+ instance_double(Project, pages_https_only?: pages_https_only)
+ end
+
+ let(:pages_domain) do
+ build(:pages_domain, certificate: certificate, key: key).tap do |pd|
+ allow(pd).to receive(:project).and_return(project)
+ pd.valid?
end
+ end
- let(:domain) { value }
+ where(:pages_https_only, :certificate, :key, :errors_on) do
+ attributes = attributes_for(:pages_domain)
+ cert, key = attributes.fetch_values(:certificate, :key)
+
+ true | nil | nil | %i(certificate key)
+ true | cert | nil | %i(key)
+ true | nil | key | %i(certificate key)
+ true | cert | key | []
+ false | nil | nil | []
+ false | cert | nil | %i(key)
+ false | nil | key | %i(key)
+ false | cert | key | []
+ end
- it { expect(pages_domain.valid?).to eq(validity) }
+ with_them do
+ it "is adds the expected errors" do
+ expect(pages_domain.errors.keys).to eq errors_on
+ end
end
end
end
@@ -43,26 +82,26 @@ describe PagesDomain do
describe 'validate certificate' do
subject { domain }
- context 'when only certificate is specified' do
- let(:domain) { build(:pages_domain, :with_certificate) }
+ context 'with matching key' do
+ let(:domain) { build(:pages_domain) }
- it { is_expected.not_to be_valid }
+ it { is_expected.to be_valid }
end
- context 'when only key is specified' do
- let(:domain) { build(:pages_domain, :with_key) }
+ context 'when no certificate is specified' do
+ let(:domain) { build(:pages_domain, :without_certificate) }
it { is_expected.not_to be_valid }
end
- context 'with matching key' do
- let(:domain) { build(:pages_domain, :with_certificate, :with_key) }
+ context 'when no key is specified' do
+ let(:domain) { build(:pages_domain, :without_key) }
- it { is_expected.to be_valid }
+ it { is_expected.not_to be_valid }
end
context 'for not matching key' do
- let(:domain) { build(:pages_domain, :with_missing_chain, :with_key) }
+ let(:domain) { build(:pages_domain, :with_missing_chain) }
it { is_expected.not_to be_valid }
end
@@ -103,30 +142,26 @@ describe PagesDomain do
describe '#url' do
subject { domain.url }
- context 'without the certificate' do
- let(:domain) { build(:pages_domain, certificate: '') }
+ let(:domain) { build(:pages_domain) }
- it { is_expected.to eq("http://#{domain.domain}") }
- end
+ it { is_expected.to eq("https://#{domain.domain}") }
- context 'with a certificate' do
- let(:domain) { build(:pages_domain, :with_certificate) }
+ context 'without the certificate' do
+ let(:domain) { build(:pages_domain, :without_certificate) }
- it { is_expected.to eq("https://#{domain.domain}") }
+ it { is_expected.to eq("http://#{domain.domain}") }
end
end
describe '#has_matching_key?' do
subject { domain.has_matching_key? }
- context 'for matching key' do
- let(:domain) { build(:pages_domain, :with_certificate, :with_key) }
+ let(:domain) { build(:pages_domain) }
- it { is_expected.to be_truthy }
- end
+ it { is_expected.to be_truthy }
context 'for invalid key' do
- let(:domain) { build(:pages_domain, :with_missing_chain, :with_key) }
+ let(:domain) { build(:pages_domain, :with_missing_chain) }
it { is_expected.to be_falsey }
end
@@ -136,7 +171,7 @@ describe PagesDomain do
subject { domain.has_intermediates? }
context 'for self signed' do
- let(:domain) { build(:pages_domain, :with_certificate) }
+ let(:domain) { build(:pages_domain) }
it { is_expected.to be_truthy }
end
@@ -162,7 +197,7 @@ describe PagesDomain do
subject { domain.expired? }
context 'for valid' do
- let(:domain) { build(:pages_domain, :with_certificate) }
+ let(:domain) { build(:pages_domain) }
it { is_expected.to be_falsey }
end
@@ -175,7 +210,7 @@ describe PagesDomain do
end
describe '#subject' do
- let(:domain) { build(:pages_domain, :with_certificate) }
+ let(:domain) { build(:pages_domain) }
subject { domain.subject }
@@ -183,7 +218,7 @@ describe PagesDomain do
end
describe '#certificate_text' do
- let(:domain) { build(:pages_domain, :with_certificate) }
+ let(:domain) { build(:pages_domain) }
subject { domain.certificate_text }
@@ -191,6 +226,18 @@ describe PagesDomain do
it { is_expected.not_to be_empty }
end
+ describe "#https?" do
+ context "when a certificate is present" do
+ subject { build(:pages_domain) }
+ it { is_expected.to be_https }
+ end
+
+ context "when no certificate is present" do
+ subject { build(:pages_domain, :without_certificate) }
+ it { is_expected.not_to be_https }
+ end
+ end
+
describe '#update_daemon' do
it 'runs when the domain is created' do
domain = build(:pages_domain)
@@ -267,29 +314,30 @@ describe PagesDomain do
end
context 'TLS configuration' do
- set(:domain_with_tls) { create(:pages_domain, :with_key, :with_certificate) }
+ set(:domain_without_tls) { create(:pages_domain, :without_certificate, :without_key) }
+ set(:domain) { create(:pages_domain) }
- let(:cert1) { domain_with_tls.certificate }
+ let(:cert1) { domain.certificate }
let(:cert2) { cert1 + ' ' }
- let(:key1) { domain_with_tls.key }
+ let(:key1) { domain.key }
let(:key2) { key1 + ' ' }
it 'updates when added' do
- expect(domain).to receive(:update_daemon)
+ expect(domain_without_tls).to receive(:update_daemon)
- domain.update!(key: key1, certificate: cert1)
+ domain_without_tls.update!(key: key1, certificate: cert1)
end
it 'updates when changed' do
- expect(domain_with_tls).to receive(:update_daemon)
+ expect(domain).to receive(:update_daemon)
- domain_with_tls.update!(key: key2, certificate: cert2)
+ domain.update!(key: key2, certificate: cert2)
end
it 'updates when removed' do
- expect(domain_with_tls).to receive(:update_daemon)
+ expect(domain).to receive(:update_daemon)
- domain_with_tls.update!(key: nil, certificate: nil)
+ domain.update!(key: nil, certificate: nil)
end
end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 61aadddceb5..1a7a6e035ea 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -3479,4 +3479,49 @@ describe Project do
end
end
end
+
+ describe "#pages_https_only?" do
+ subject { build(:project) }
+
+ context "when HTTPS pages are disabled" do
+ it { is_expected.not_to be_pages_https_only }
+ end
+
+ context "when HTTPS pages are enabled", :https_pages_enabled do
+ it { is_expected.to be_pages_https_only }
+ end
+ end
+
+ describe "#pages_https_only? validation", :https_pages_enabled do
+ subject(:project) do
+ # set-up dirty object:
+ create(:project, pages_https_only: false).tap do |p|
+ p.pages_https_only = true
+ end
+ end
+
+ context "when no domains are associated" do
+ it { is_expected.to be_valid }
+ end
+
+ context "when domains including keys and certificates are associated" do
+ before do
+ allow(project)
+ .to receive(:pages_domains)
+ .and_return([instance_double(PagesDomain, https?: true)])
+ end
+
+ it { is_expected.to be_valid }
+ end
+
+ context "when domains including no keys or certificates are associated" do
+ before do
+ allow(project)
+ .to receive(:pages_domains)
+ .and_return([instance_double(PagesDomain, https?: false)])
+ end
+
+ it { is_expected.not_to be_valid }
+ end
+ end
end
diff --git a/spec/requests/api/pages_domains_spec.rb b/spec/requests/api/pages_domains_spec.rb
index dc3a116c060..a9ccbb32666 100644
--- a/spec/requests/api/pages_domains_spec.rb
+++ b/spec/requests/api/pages_domains_spec.rb
@@ -1,17 +1,17 @@
require 'rails_helper'
describe API::PagesDomains do
- set(:project) { create(:project, path: 'my.project') }
+ set(:project) { create(:project, path: 'my.project', pages_https_only: false) }
set(:user) { create(:user) }
set(:admin) { create(:admin) }
- set(:pages_domain) { create(:pages_domain, domain: 'www.domain.test', project: project) }
- set(:pages_domain_secure) { create(:pages_domain, :with_certificate, :with_key, domain: 'ssl.domain.test', project: project) }
- set(:pages_domain_expired) { create(:pages_domain, :with_expired_certificate, :with_key, domain: 'expired.domain.test', project: project) }
+ set(:pages_domain) { create(:pages_domain, :without_key, :without_certificate, domain: 'www.domain.test', project: project) }
+ set(:pages_domain_secure) { create(:pages_domain, domain: 'ssl.domain.test', project: project) }
+ set(:pages_domain_expired) { create(:pages_domain, :with_expired_certificate, domain: 'expired.domain.test', project: project) }
- let(:pages_domain_params) { build(:pages_domain, domain: 'www.other-domain.test').slice(:domain) }
- let(:pages_domain_secure_params) { build(:pages_domain, :with_certificate, :with_key, domain: 'ssl.other-domain.test', project: project).slice(:domain, :certificate, :key) }
- let(:pages_domain_secure_key_missmatch_params) {build(:pages_domain, :with_trusted_chain, :with_key, project: project).slice(:domain, :certificate, :key) }
+ let(:pages_domain_params) { build(:pages_domain, :without_key, :without_certificate, domain: 'www.other-domain.test').slice(:domain) }
+ let(:pages_domain_secure_params) { build(:pages_domain, domain: 'ssl.other-domain.test', project: project).slice(:domain, :certificate, :key) }
+ let(:pages_domain_secure_key_missmatch_params) {build(:pages_domain, :with_trusted_chain, project: project).slice(:domain, :certificate, :key) }
let(:pages_domain_secure_missing_chain_params) {build(:pages_domain, :with_missing_chain, project: project).slice(:certificate) }
let(:route) { "/projects/#{project.id}/pages/domains" }
diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb
index e3b49a6242d..f48d466d263 100644
--- a/spec/services/projects/update_service_spec.rb
+++ b/spec/services/projects/update_service_spec.rb
@@ -241,6 +241,27 @@ describe Projects::UpdateService do
})
end
end
+
+ context 'when updating #pages_https_only', :https_pages_enabled do
+ subject(:call_service) do
+ update_project(project, admin, pages_https_only: false)
+ end
+
+ it 'updates the attribute' do
+ expect { call_service }
+ .to change { project.pages_https_only? }
+ .to(false)
+ end
+
+ it 'calls Projects::UpdatePagesConfigurationService' do
+ expect(Projects::UpdatePagesConfigurationService)
+ .to receive(:new)
+ .with(project)
+ .and_call_original
+
+ call_service
+ end
+ end
end
describe '#run_auto_devops_pipeline?' do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 9f6f0204a16..5051cd34564 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -197,6 +197,22 @@ RSpec.configure do |config|
Ability.allowed?(*args)
end
end
+
+ config.before(:each, :http_pages_enabled) do |_|
+ allow(Gitlab.config.pages).to receive(:external_http).and_return(['1.1.1.1:80'])
+ end
+
+ config.before(:each, :https_pages_enabled) do |_|
+ allow(Gitlab.config.pages).to receive(:external_https).and_return(['1.1.1.1:443'])
+ end
+
+ config.before(:each, :http_pages_disabled) do |_|
+ allow(Gitlab.config.pages).to receive(:external_http).and_return(false)
+ end
+
+ config.before(:each, :https_pages_disabled) do |_|
+ allow(Gitlab.config.pages).to receive(:external_https).and_return(false)
+ end
end
# add simpler way to match asset paths containing digest strings