diff --git a/app/assets/javascripts/pages/projects/pages_domains/form.js b/app/assets/javascripts/pages/projects/pages_domains/form.js index 1d0dbfe0406..cef8e92610c 100644 --- a/app/assets/javascripts/pages/projects/pages_domains/form.js +++ b/app/assets/javascripts/pages/projects/pages_domains/form.js @@ -5,14 +5,6 @@ export default () => { if (toggleContainer) { const onToggleButtonClicked = isAutoSslEnabled => { - Array.from(document.querySelectorAll('.js-shown-if-auto-ssl')).forEach(el => { - if (isAutoSslEnabled) { - el.classList.remove('d-none'); - } else { - el.classList.add('d-none'); - } - }); - Array.from(document.querySelectorAll('.js-shown-unless-auto-ssl')).forEach(el => { if (isAutoSslEnabled) { el.classList.add('d-none'); @@ -21,14 +13,6 @@ export default () => { } }); - Array.from(document.querySelectorAll('.js-enabled-if-auto-ssl')).forEach(el => { - if (isAutoSslEnabled) { - el.removeAttribute('disabled'); - } else { - el.setAttribute('disabled', 'disabled'); - } - }); - Array.from(document.querySelectorAll('.js-enabled-unless-auto-ssl')).forEach(el => { if (isAutoSslEnabled) { el.setAttribute('disabled', 'disabled'); diff --git a/app/controllers/projects/pages_domains_controller.rb b/app/controllers/projects/pages_domains_controller.rb index 89f21d8dadb..c287e440db0 100644 --- a/app/controllers/projects/pages_domains_controller.rb +++ b/app/controllers/projects/pages_domains_controller.rb @@ -65,16 +65,14 @@ class Projects::PagesDomainsController < Projects::ApplicationController private def create_params - params.require(:pages_domain).permit(:key, :certificate, :domain, :auto_ssl_enabled) + params.require(:pages_domain).permit(:user_provided_key, :user_provided_certificate, :domain, :auto_ssl_enabled) end def update_params - params.require(:pages_domain).permit(:key, :certificate, :auto_ssl_enabled) + params.require(:pages_domain).permit(:user_provided_key, :user_provided_certificate, :auto_ssl_enabled) end - # rubocop: disable CodeReuse/ActiveRecord def domain - @domain ||= @project.pages_domains.find_by!(domain: params[:id].to_s) + @domain ||= @project.pages_domains.find_by_domain!(params[:id].to_s) end - # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index 524df30289e..07195c0bfd3 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -4,6 +4,8 @@ class PagesDomain < ApplicationRecord VERIFICATION_KEY = 'gitlab-pages-verification-code'.freeze VERIFICATION_THRESHOLD = 3.days.freeze + enum certificate_source: { user_provided: 0, gitlab_provided: 1 }, _prefix: :certificate + belongs_to :project has_many :acme_orders, class_name: "PagesDomainAcmeOrder" @@ -143,6 +145,34 @@ class PagesDomain < ApplicationRecord self.certificate_valid_not_after = x509&.not_after end + def user_provided_key + key if certificate_user_provided? + end + + def user_provided_key=(key) + self.key = key + self.certificate_source = 'user_provided' if key_changed? + end + + def user_provided_certificate + certificate if certificate_user_provided? + end + + def user_provided_certificate=(certificate) + self.certificate = certificate + self.certificate_source = 'user_provided' if certificate_changed? + end + + def gitlab_provided_certificate=(certificate) + self.certificate = certificate + self.certificate_source = 'gitlab_provided' if certificate_changed? + end + + def gitlab_provided_key=(key) + self.key = key + self.certificate_source = 'gitlab_provided' if key_changed? + end + private def set_verification_code diff --git a/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb b/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb index 2dfe1a3d8ca..3413a9e4612 100644 --- a/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb +++ b/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb @@ -35,7 +35,7 @@ module PagesDomains def save_certificate(private_key, api_order) certificate = api_order.certificate - pages_domain.update!(key: private_key, certificate: certificate) + pages_domain.update!(gitlab_provided_key: private_key, gitlab_provided_certificate: certificate) end end end diff --git a/app/views/projects/pages_domains/_form.html.haml b/app/views/projects/pages_domains/_form.html.haml index 33f2166480b..e7edb93f05b 100644 --- a/app/views/projects/pages_domains/_form.html.haml +++ b/app/views/projects/pages_domains/_form.html.haml @@ -11,7 +11,7 @@ - if Gitlab.config.pages.external_https - - auto_ssl_available = Feature.enabled?(:pages_auto_ssl) + - auto_ssl_available = ::Gitlab::LetsEncrypt::Client.new.enabled? - auto_ssl_enabled = @domain.auto_ssl_enabled? - auto_ssl_available_and_enabled = auto_ssl_available && auto_ssl_enabled @@ -38,40 +38,24 @@ - docs_link_end = "".html_safe = _("Let's Encrypt is a free, automated, and open certificate authority (CA) that gives digital certificates in order to enable HTTPS (SSL/TLS) for websites. Learn more about Let's Encrypt configuration by following the %{docs_link_start}documentation on GitLab Pages%{docs_link_end}.").html_safe % { docs_link_url: docs_link_url, docs_link_start: docs_link_start, docs_link_end: docs_link_end } - .js-shown-if-auto-ssl{ class: ("d-none" unless auto_ssl_available_and_enabled) } - .form-group.row - .col-sm-2.col-form-label - = f.label :certificate, _("Certificate (PEM)") - .col-sm-10 - - if auto_ssl_available_and_enabled && !@domain.certificate.empty? - = f.text_area :certificate, - rows: 5, - class: "form-control", - disabled: true - %span.help-inline.text-muted= _("This certificate is automatically managed by Let's Encrypt") - - else - %p.text-secondary.form-control-plaintext= _("The certificate will be shown here once it has been obtained from Let's Encrypt. This process may take up to an hour to complete.") - .js-shown-unless-auto-ssl{ class: ("d-none" if auto_ssl_available_and_enabled) } .form-group.row .col-sm-2.col-form-label - = f.label :certificate, _("Certificate (PEM)") + = f.label :user_provided_certificate, _("Certificate (PEM)") .col-sm-10 - = f.text_area :certificate, + = f.text_area :user_provided_certificate, rows: 5, class: "form-control js-enabled-unless-auto-ssl", - value: (@domain.certificate unless auto_ssl_available_and_enabled), disabled: auto_ssl_available_and_enabled %span.help-inline.text-muted= _("Upload a certificate for your domain with all intermediates") .form-group.row .col-sm-2.col-form-label - = f.label :key, _("Key (PEM)") + = f.label :user_provided_key, _("Key (PEM)") .col-sm-10 - = f.text_area :key, + = f.text_area :user_provided_key, rows: 5, class: "form-control js-enabled-unless-auto-ssl", - value: (@domain.key unless auto_ssl_available_and_enabled), disabled: auto_ssl_available_and_enabled %span.help-inline.text-muted= _("Upload a private key for your certificate") diff --git a/db/migrate/20190607085356_add_source_to_pages_domains.rb b/db/migrate/20190607085356_add_source_to_pages_domains.rb new file mode 100644 index 00000000000..0a845d7d11f --- /dev/null +++ b/db/migrate/20190607085356_add_source_to_pages_domains.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddSourceToPagesDomains < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:pages_domains, :certificate_source, :smallint, default: 0) + end + + def down + remove_column(:pages_domains, :certificate_source) + end +end diff --git a/db/schema.rb b/db/schema.rb index 02d8ab10935..c1c67e012e9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2333,6 +2333,7 @@ ActiveRecord::Schema.define(version: 20190619175843) do t.boolean "auto_ssl_enabled", default: false, null: false t.datetime_with_timezone "certificate_valid_not_before" t.datetime_with_timezone "certificate_valid_not_after" + t.integer "certificate_source", limit: 2, default: 0, null: false t.index ["domain"], name: "index_pages_domains_on_domain", unique: true, using: :btree t.index ["project_id", "enabled_until"], name: "index_pages_domains_on_project_id_and_enabled_until", using: :btree t.index ["project_id"], name: "index_pages_domains_on_project_id", using: :btree diff --git a/lib/api/pages_domains.rb b/lib/api/pages_domains.rb index 78442f465bd..4227a106a95 100644 --- a/lib/api/pages_domains.rb +++ b/lib/api/pages_domains.rb @@ -90,14 +90,15 @@ module API end params do requires :domain, type: String, desc: 'The domain' - optional :certificate, allow_blank: false, types: [File, String], desc: 'The certificate' - optional :key, allow_blank: false, types: [File, String], desc: 'The key' - all_or_none_of :certificate, :key + optional :certificate, allow_blank: false, types: [File, String], desc: 'The certificate', as: :user_provided_certificate + optional :key, allow_blank: false, types: [File, String], desc: 'The key', as: :user_provided_key + all_or_none_of :user_provided_certificate, :user_provided_key end post ":id/pages/domains" do authorize! :update_pages, user_project pages_domain_params = declared(params, include_parent_namespaces: false) + pages_domain = user_project.pages_domains.create(pages_domain_params) if pages_domain.persisted? @@ -110,8 +111,8 @@ module API desc 'Updates a pages domain' params do requires :domain, type: String, desc: 'The domain' - optional :certificate, allow_blank: false, types: [File, String], desc: 'The certificate' - optional :key, allow_blank: false, types: [File, String], desc: 'The key' + optional :certificate, allow_blank: false, types: [File, String], desc: 'The certificate', as: :user_provided_certificate + optional :key, allow_blank: false, types: [File, String], desc: 'The key', as: :user_provided_key end put ":id/pages/domains/:domain", requirements: PAGES_DOMAINS_ENDPOINT_REQUIREMENTS do authorize! :update_pages, user_project @@ -119,8 +120,8 @@ module API pages_domain_params = declared(params, include_parent_namespaces: false) # Remove empty private key if certificate is not empty. - if pages_domain_params[:certificate] && !pages_domain_params[:key] - pages_domain_params.delete(:key) + if pages_domain_params[:user_provided_certificate] && !pages_domain_params[:user_provided_key] + pages_domain_params.delete(:user_provided_key) end if pages_domain.update(pages_domain_params) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8f010b2184c..84f785de0c2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9942,9 +9942,6 @@ msgstr "" msgid "The X509 Certificate to use when mutual TLS is required to communicate with the external authorization service. If left blank, the server certificate is still validated when accessing over HTTPS." msgstr "" -msgid "The certificate will be shown here once it has been obtained from Let's Encrypt. This process may take up to an hour to complete." -msgstr "" - msgid "The character highlighter helps you keep the subject line to %{titleLength} characters and wrap the body at %{bodyLength} so they are readable in git." msgstr "" @@ -10278,9 +10275,6 @@ msgstr "" msgid "This branch has changed since you started editing. Would you like to create a new branch?" msgstr "" -msgid "This certificate is automatically managed by Let's Encrypt" -msgstr "" - msgid "This commit is part of merge request %{link_to_merge_request}. Comments created here will be created in the context of that merge request." msgstr "" diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb index ff3afd51cd8..032f4f1418f 100644 --- a/spec/controllers/projects/pages_domains_controller_spec.rb +++ b/spec/controllers/projects/pages_domains_controller_spec.rb @@ -15,7 +15,10 @@ describe Projects::PagesDomainsController do end let(:pages_domain_params) do - build(:pages_domain, domain: 'my.otherdomain.com').slice(:key, :certificate, :domain) + attributes_for(:pages_domain, domain: 'my.otherdomain.com').slice(:key, :certificate, :domain).tap do |params| + params[:user_provided_key] = params.delete(:key) + params[:user_provided_certificate] = params.delete(:certificate) + end end before do @@ -84,48 +87,59 @@ describe Projects::PagesDomainsController do controller.instance_variable_set(:@domain, pages_domain) end - let(:pages_domain_params) do - attributes_for(:pages_domain).slice(:key, :certificate) - end - let(:params) do request_params.merge(id: pages_domain.domain, pages_domain: pages_domain_params) end - it 'updates the domain' do - expect(pages_domain) - .to receive(:update) - .with(ActionController::Parameters.new(pages_domain_params).permit!) - .and_return(true) + context 'with valid params' do + let(:pages_domain_params) do + attributes_for(:pages_domain, :with_trusted_chain).slice(:key, :certificate).tap do |params| + params[:user_provided_key] = params.delete(:key) + params[:user_provided_certificate] = params.delete(:certificate) + end + end - patch(:update, params: params) + it 'updates the domain' do + expect do + patch(:update, params: params) + end.to change { pages_domain.reload.certificate }.to(pages_domain_params[:user_provided_certificate]) + end + + it 'redirects to the project page' do + patch(:update, params: params) + + expect(flash[:notice]).to eq 'Domain was updated' + expect(response).to redirect_to(project_pages_path(project)) + end end - it 'redirects to the project page' do - patch(:update, params: params) + context 'with key parameter' do + before do + pages_domain.update!(key: nil, certificate: nil, certificate_source: 'gitlab_provided') + end - expect(flash[:notice]).to eq 'Domain was updated' - expect(response).to redirect_to(project_pages_path(project)) + it 'marks certificate as provided by user' do + expect do + patch(:update, params: params) + end.to change { pages_domain.reload.certificate_source }.from('gitlab_provided').to('user_provided') + end end context 'the domain is invalid' do - it 'renders the edit action' do - allow(pages_domain).to receive(:update).and_return(false) + let(:pages_domain_params) { { user_provided_certificate: 'blabla' } } + it 'renders the edit action' do patch(:update, params: params) expect(response).to render_template('edit') end end - context 'the parameters include the domain' do - it 'renders 400 Bad Request' do - expect(pages_domain) - .to receive(:update) - .with(hash_not_including(:domain)) - .and_return(true) - - patch(:update, params: params.deep_merge(pages_domain: { domain: 'abc' })) + context 'when parameters include the domain' do + it 'does not update domain' do + expect do + patch(:update, params: params.deep_merge(pages_domain: { domain: 'abc' })) + end.not_to change { pages_domain.reload.domain } end end end diff --git a/spec/factories/pages_domains.rb b/spec/factories/pages_domains.rb index db8384877b0..8da19a37a6a 100644 --- a/spec/factories/pages_domains.rb +++ b/spec/factories/pages_domains.rb @@ -180,5 +180,9 @@ Iy6oRpHaCF/2obZdIdgf9rlyz0fkqyHJc9GkioSoOhJZxEV2SgAkap8yS0sX2tJ9 ZDXgrA== -----END CERTIFICATE-----' end + + trait :letsencrypt do + certificate_source { :gitlab_provided } + end end end diff --git a/spec/features/projects/pages_lets_encrypt_spec.rb b/spec/features/projects/pages_lets_encrypt_spec.rb index baa217cbe58..a5f8702302c 100644 --- a/spec/features/projects/pages_lets_encrypt_spec.rb +++ b/spec/features/projects/pages_lets_encrypt_spec.rb @@ -2,124 +2,119 @@ require 'spec_helper' describe "Pages with Let's Encrypt", :https_pages_enabled do + include LetsEncryptHelpers + let(:project) { create(:project) } let(:user) { create(:user) } let(:role) { :maintainer } - let(:certificate_pem) do - <<~PEM - -----BEGIN CERTIFICATE----- - MIICGzCCAYSgAwIBAgIBATANBgkqhkiG9w0BAQUFADAbMRkwFwYDVQQDExB0ZXN0 - LWNlcnRpZmljYXRlMB4XDTE2MDIxMjE0MzIwMFoXDTIwMDQxMjE0MzIwMFowGzEZ - MBcGA1UEAxMQdGVzdC1jZXJ0aWZpY2F0ZTCBnzANBgkqhkiG9w0BAQEFAAOBjQAw - gYkCgYEApL4J9L0ZxFJ1hI1LPIflAlAGvm6ZEvoT4qKU5Xf2JgU7/2geNR1qlNFa - SvCc08Knupp5yTgmvyK/Xi09U0N82vvp4Zvr/diSc4A/RA6Mta6egLySNT438kdT - nY2tR5feoTLwQpX0t4IMlwGQGT5h6Of2fKmDxzuwuyffcIHqLdsCAwEAAaNvMG0w - DAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUxl9WSxBprB0z0ibJs3rXEk0+95AwCwYD - VR0PBAQDAgXgMBEGCWCGSAGG+EIBAQQEAwIGQDAeBglghkgBhvhCAQ0EERYPeGNh - IGNlcnRpZmljYXRlMA0GCSqGSIb3DQEBBQUAA4GBAGC4T8SlFHK0yPSa+idGLQFQ - joZp2JHYvNlTPkRJ/J4TcXxBTJmArcQgTIuNoBtC+0A/SwdK4MfTCUY4vNWNdese - 5A4K65Nb7Oh1AdQieTBHNXXCdyFsva9/ScfQGEl7p55a52jOPs0StPd7g64uvjlg - YHi2yesCrOvVXt+lgPTd - -----END CERTIFICATE----- - PEM - end + let(:certificate_pem) { attributes_for(:pages_domain)[:certificate] } - let(:certificate_key) do - <<~KEY - -----BEGIN PRIVATE KEY----- - MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKS+CfS9GcRSdYSN - SzyH5QJQBr5umRL6E+KilOV39iYFO/9oHjUdapTRWkrwnNPCp7qaeck4Jr8iv14t - PVNDfNr76eGb6/3YknOAP0QOjLWunoC8kjU+N/JHU52NrUeX3qEy8EKV9LeCDJcB - kBk+Yejn9nypg8c7sLsn33CB6i3bAgMBAAECgYA2D26w80T7WZvazYr86BNMePpd - j2mIAqx32KZHzt/lhh40J/SRtX9+Kl0Y7nBoRR5Ja9u/HkAIxNxLiUjwg9r6cpg/ - uITEF5nMt7lAk391BuI+7VOZZGbJDsq2ulPd6lO+C8Kq/PI/e4kXcIjeH6KwQsuR - 5vrXfBZ3sQfflaiN4QJBANBt8JY2LIGQF8o89qwUpRL5vbnKQ4IzZ5+TOl4RLR7O - AQpJ81tGuINghO7aunctb6rrcKJrxmEH1whzComybrMCQQDKV49nOBudRBAIgG4K - EnLzsRKISUHMZSJiYTYnablof8cKw1JaQduw7zgrUlLwnroSaAGX88+Jw1f5n2Lh - Vlg5AkBDdUGnrDLtYBCDEQYZHblrkc7ZAeCllDOWjxUV+uMqlCv8A4Ey6omvY57C - m6I8DkWVAQx8VPtozhvHjUw80rZHAkB55HWHAM3h13axKG0htCt7klhPsZHpx6MH - EPjGlXIT+aW2XiPmK3ZlCDcWIenE+lmtbOpI159Wpk8BGXs/s/xBAkEAlAY3ymgx - 63BDJEwvOb2IaP8lDDxNsXx9XJNVvQbv5n15vNsLHbjslHfAhAbxnLQ1fLhUPqSi - nNp/xedE1YxutQ== - -----END PRIVATE KEY----- - KEY - end + let(:certificate_key) { attributes_for(:pages_domain)[:key] } before do allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + stub_lets_encrypt_settings + project.add_role(user, role) sign_in(user) project.namespace.update(owner: user) allow_any_instance_of(Project).to receive(:pages_deployed?) { true } end - context 'when the page_auto_ssl feature flag is enabled' do + context 'when the auto SSL management is initially disabled' do + let(:domain) do + create(:pages_domain, auto_ssl_enabled: false, project: project) + end + + it 'enables auto SSL and dynamically updates the form accordingly', :js do + visit edit_project_pages_domain_path(project, domain) + + expect(domain.auto_ssl_enabled).to eq false + + expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'false' + expect(page).to have_field 'Certificate (PEM)', type: 'textarea' + expect(page).to have_field 'Key (PEM)', type: 'textarea' + + find('.js-auto-ssl-toggle-container .project-feature-toggle').click + + expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'true' + expect(page).not_to have_field 'Certificate (PEM)', type: 'textarea' + expect(page).not_to have_field 'Key (PEM)', type: 'textarea' + + click_on 'Save Changes' + + expect(domain.reload.auto_ssl_enabled).to eq true + end + end + + context 'when the auto SSL management is initially enabled' do + let(:domain) do + create(:pages_domain, :letsencrypt, auto_ssl_enabled: true, project: project) + end + + it 'disables auto SSL and dynamically updates the form accordingly', :js do + visit edit_project_pages_domain_path(project, domain) + + expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'true' + expect(page).not_to have_field 'Certificate (PEM)', type: 'textarea' + expect(page).not_to have_field 'Key (PEM)', type: 'textarea' + + find('.js-auto-ssl-toggle-container .project-feature-toggle').click + + expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'false' + expect(page).to have_field 'Certificate (PEM)', type: 'textarea' + expect(page).to have_field 'Key (PEM)', type: 'textarea' + + fill_in 'Certificate (PEM)', with: certificate_pem + fill_in 'Key (PEM)', with: certificate_key + + click_on 'Save Changes' + + expect(domain.reload.auto_ssl_enabled).to eq false + end + end + + shared_examples 'user sees private keys only for user provided certificate' do before do - stub_feature_flags(pages_auto_ssl: true) + visit edit_project_pages_domain_path(project, domain) end - context 'when the auto SSL management is initially disabled' do - let(:domain) do - create(:pages_domain, auto_ssl_enabled: false, project: project) - end - - it 'enables auto SSL and dynamically updates the form accordingly', :js do - visit edit_project_pages_domain_path(project, domain) - - expect(domain.auto_ssl_enabled).to eq false - - expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'false' - expect(page).to have_field 'Certificate (PEM)', type: 'textarea' - expect(page).to have_field 'Key (PEM)', type: 'textarea' - - find('.js-auto-ssl-toggle-container .project-feature-toggle').click - - expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'true' - expect(page).not_to have_field 'Certificate (PEM)', type: 'textarea' - expect(page).not_to have_field 'Key (PEM)', type: 'textarea' - expect(page).to have_content "The certificate will be shown here once it has been obtained from Let's Encrypt. This process may take up to an hour to complete." - - click_on 'Save Changes' - - expect(domain.reload.auto_ssl_enabled).to eq true + shared_examples 'user do not see private key' do + it 'user do not see private key' do + expect(find_field('Key (PEM)', visible: :all, disabled: :all).value).to be_blank end end - context 'when the auto SSL management is initially enabled' do - let(:domain) do - create(:pages_domain, auto_ssl_enabled: true, project: project) - end + context 'when auto_ssl is enabled for domain' do + let(:domain) { create(:pages_domain, :letsencrypt, project: project, auto_ssl_enabled: true) } - it 'disables auto SSL and dynamically updates the form accordingly', :js do - visit edit_project_pages_domain_path(project, domain) + include_examples 'user do not see private key' + end - expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'true' - expect(page).to have_field 'Certificate (PEM)', type: 'textarea', disabled: true - expect(page).not_to have_field 'Key (PEM)', type: 'textarea' + context 'when auto_ssl is disabled for domain' do + let(:domain) { create(:pages_domain, :letsencrypt, project: project) } - find('.js-auto-ssl-toggle-container .project-feature-toggle').click + include_examples 'user do not see private key' + end - expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'false' - expect(page).to have_field 'Certificate (PEM)', type: 'textarea' - expect(page).to have_field 'Key (PEM)', type: 'textarea' + context 'when certificate is provided by user' do + let(:domain) { create(:pages_domain, project: project) } - fill_in 'Certificate (PEM)', with: certificate_pem - fill_in 'Key (PEM)', with: certificate_key - - click_on 'Save Changes' - - expect(domain.reload.auto_ssl_enabled).to eq false + it 'user sees private key' do + expect(find_field('Key (PEM)').value).not_to be_blank end end end - context 'when the page_auto_ssl feature flag is disabled' do + include_examples 'user sees private keys only for user provided certificate' + + context 'when letsencrypt is disabled' do let(:domain) do create(:pages_domain, auto_ssl_enabled: false, project: project) end before do - stub_feature_flags(pages_auto_ssl: false) + stub_application_setting(lets_encrypt_terms_of_service_accepted: false) visit edit_project_pages_domain_path(project, domain) end @@ -127,5 +122,7 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do it "does not render the Let's Encrypt field", :js do expect(page).not_to have_selector '.js-auto-ssl-toggle-container' end + + include_examples 'user sees private keys only for user provided certificate' end end diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb index 9bb0ba81ef5..c4b3ddb2088 100644 --- a/spec/features/projects/pages_spec.rb +++ b/spec/features/projects/pages_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -shared_examples 'pages domain editing' do +shared_examples 'pages settings editing' do let(:project) { create(:project) } let(:user) { create(:user) } let(:role) { :maintainer } @@ -321,19 +321,15 @@ shared_examples 'pages domain editing' do end describe 'Pages' do - context 'when pages_auto_ssl feature flag is disabled' do + include LetsEncryptHelpers + + include_examples 'pages settings editing' + + context 'when letsencrypt support is enabled' do before do - stub_feature_flags(pages_auto_ssl: false) + stub_lets_encrypt_settings end - include_examples 'pages domain editing' - end - - context 'when pages_auto_ssl feature flag is enabled' do - before do - stub_feature_flags(pages_auto_ssl: true) - end - - include_examples 'pages domain editing' + include_examples 'pages settings editing' end end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index fdc81359d34..4fb7b71a3c7 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -356,6 +356,102 @@ describe PagesDomain do end end + describe '#user_provided_key' do + subject { domain.user_provided_key } + + context 'when certificate is provided by user' do + let(:domain) { create(:pages_domain) } + + it 'returns key' do + is_expected.to eq(domain.key) + end + end + + context 'when certificate is provided by gitlab' do + let(:domain) { create(:pages_domain, :letsencrypt) } + + it 'returns nil' do + is_expected.to be_nil + end + end + end + + describe '#user_provided_certificate' do + subject { domain.user_provided_certificate } + + context 'when certificate is provided by user' do + let(:domain) { create(:pages_domain) } + + it 'returns key' do + is_expected.to eq(domain.certificate) + end + end + + context 'when certificate is provided by gitlab' do + let(:domain) { create(:pages_domain, :letsencrypt) } + + it 'returns nil' do + is_expected.to be_nil + end + end + end + + shared_examples 'certificate setter' do |attribute, setter_name, old_certificate_source, new_certificate_source| + let(:domain) do + create(:pages_domain, certificate_source: old_certificate_source) + end + + let(:old_value) { domain.public_send(attribute) } + + subject { domain.public_send(setter_name, new_value) } + + context 'when value has been changed' do + let(:new_value) { 'new_value' } + + it "assignes new value to #{attribute}" do + expect do + subject + end.to change { domain.public_send(attribute) }.from(old_value).to('new_value') + end + + it 'changes certificate source' do + expect do + subject + end.to change { domain.certificate_source }.from(old_certificate_source).to(new_certificate_source) + end + end + + context 'when value has not been not changed' do + let(:new_value) { old_value } + + it 'does not change certificate source' do + expect do + subject + end.not_to change { domain.certificate_source }.from(old_certificate_source) + end + end + end + + describe '#user_provided_key=' do + include_examples('certificate setter', 'key', 'user_provided_key=', + 'gitlab_provided', 'user_provided') + end + + describe '#gitlab_provided_key=' do + include_examples('certificate setter', 'key', 'gitlab_provided_key=', + 'user_provided', 'gitlab_provided') + end + + describe '#user_provided_certificate=' do + include_examples('certificate setter', 'certificate', 'user_provided_certificate=', + 'gitlab_provided', 'user_provided') + end + + describe '#gitlab_provided_certificate=' do + include_examples('certificate setter', 'certificate', 'gitlab_provided_certificate=', + 'user_provided', 'gitlab_provided') + end + describe '.for_removal' do subject { described_class.for_removal } diff --git a/spec/requests/api/pages_domains_spec.rb b/spec/requests/api/pages_domains_spec.rb index 3eb68a6abb6..449032b95b7 100644 --- a/spec/requests/api/pages_domains_spec.rb +++ b/spec/requests/api/pages_domains_spec.rb @@ -359,6 +359,14 @@ describe API::PagesDomains do expect(pages_domain_secure.certificate).to eq(params_secure_nokey[:certificate]) end + it 'updates certificate source to user_provided if is changed' do + pages_domain.update!(certificate_source: 'gitlab_provided') + + expect do + put api(route_domain, user), params: params_secure + end.to change { pages_domain.reload.certificate_source }.from('gitlab_provided').to('user_provided') + end + it 'fails to update pages domain adding certificate without key' do put api(route_domain, user), params: params_secure_nokey diff --git a/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb b/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb index 6d7be27939c..d5f77f3354b 100644 --- a/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb +++ b/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb @@ -137,6 +137,12 @@ describe PagesDomains::ObtainLetsEncryptCertificateService do expect(pages_domain.certificate).to eq(certificate) end + it 'marks certificate as gitlab_provided' do + service.execute + + expect(pages_domain.certificate_source).to eq("gitlab_provided") + end + it 'removes order from database' do service.execute