diff --git a/app/assets/javascripts/pages/projects/pages_domains/form.js b/app/assets/javascripts/pages/projects/pages_domains/form.js index 1d0dbfe0406e608d5d6d9592b2f585ce68b77566..cef8e92610cf70360bcd57f8eee398964e6024db 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 89f21d8dadb32c5f32d89db668f7bb4a872a7677..c287e440db06447076f37817a4b2fcd4cb3def3d 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 524df30289e2f6b5557eff51cce83d2fe152a057..07195c0bfd388a717fbfa88a8e9a9b14d89af748 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 2dfe1a3d8ca2838f7af5f2f96d47168f31ae9e18..3413a9e46120872d7e33be66cf5406ff69510790 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 33f2166480b1c9d7aa49a39e52db88be8444cf5d..e7edb93f05b0fd28246bddbce7d243d34d69e542 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 0000000000000000000000000000000000000000..0a845d7d11f1dd252cf1d774d8bc26e04dad84f4 --- /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 02d8ab10935f9fdc09d706d6ec6c24386b162641..c1c67e012e99fe7047846a4e4672a892e77ecf70 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 78442f465bd571fb2f1384dbbb3552fd3ee884da..4227a106a95ba983db5723a87eb76df34b66396b 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 8f010b2184c89deb9880b7ae04fbdf1a92738303..84f785de0c2ff5ce721fe52153d0add550ab866d 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 ff3afd51cd8a00bc6acffa5cb0eab21452e43c6f..032f4f1418f4864e761cee13b80c7442e8db1320 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 + + 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) - 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 db8384877b0c2257d99064fb9cc7ac58e7a15c98..8da19a37a6a8b25c6f18d48f358a32aa681900fc 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 baa217cbe580a8b5d70de0a72bcce6c4a90759cf..a5f8702302c9bdc8015212279419b17c05d0ce98 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 - before do - stub_feature_flags(pages_auto_ssl: true) + context 'when the auto SSL management is initially disabled' do + let(:domain) do + create(:pages_domain, auto_ssl_enabled: false, project: project) 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) - 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(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' - 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 - 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(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' - click_on 'Save Changes' + expect(domain.reload.auto_ssl_enabled).to eq true + end + end - expect(domain.reload.auto_ssl_enabled).to eq true - 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 - context 'when the auto SSL management is initially enabled' do - let(:domain) do - create(:pages_domain, 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) - 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' - 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' + find('.js-auto-ssl-toggle-container .project-feature-toggle').click - 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' - 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 - fill_in 'Certificate (PEM)', with: certificate_pem - fill_in 'Key (PEM)', with: certificate_key + click_on 'Save Changes' - click_on 'Save Changes' + expect(domain.reload.auto_ssl_enabled).to eq false + end + end - expect(domain.reload.auto_ssl_enabled).to eq false + shared_examples 'user sees private keys only for user provided certificate' do + before do + visit edit_project_pages_domain_path(project, domain) + end + + 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 auto_ssl is enabled for domain' do + let(:domain) { create(:pages_domain, :letsencrypt, project: project, auto_ssl_enabled: true) } + + include_examples 'user do not see private key' + end + + context 'when auto_ssl is disabled for domain' do + let(:domain) { create(:pages_domain, :letsencrypt, project: project) } + + include_examples 'user do not see private key' + end + + context 'when certificate is provided by user' do + let(:domain) { create(:pages_domain, project: project) } + + 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 9bb0ba81ef56bc35505b3327d4dd9a4dbfca0d3d..c4b3ddb2088043c0f933b9e327a9d72dd1092efe 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 - before do - stub_feature_flags(pages_auto_ssl: false) - end + include LetsEncryptHelpers - include_examples 'pages domain editing' - end + include_examples 'pages settings editing' - context 'when pages_auto_ssl feature flag is enabled' do + context 'when letsencrypt support is enabled' do before do - stub_feature_flags(pages_auto_ssl: true) + stub_lets_encrypt_settings 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 fdc81359d34adb5f60976c537c6a12ae6c3a0413..4fb7b71a3c77a2654f46d178356109c3dc50af0d 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 3eb68a6abb6b3994e9450cc191c96e4f9615edf0..449032b95b710b0459bb4768c4d67086f39a8ac8 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 6d7be27939c640b0268a15fe8a98c599fcbd9b87..d5f77f3354be2077021f30975c6cf68d22ce8367 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