Commit 6119d5ad authored by Vladimir Shushlin's avatar Vladimir Shushlin Committed by Nick Thomas

Don't show private keys for letsencrypt certs

Adds enum certificate_source to pages_domains table
with default manually_uploaded

Mark certificates as 'gitlab_provided'
if the were obtained through Let's Encrypt

Mark certificates as 'user_provided' if they were uploaded through
controller or api

Only show private key in domain edit form if it is 'user_provided'

Only show LetsEncrypt option if is enabled by application settings
(and feature flag)

Refactor and fix some specs to match new logic

Don't show Let's Encrypt certificates as well
parent 176164d3
...@@ -5,14 +5,6 @@ export default () => { ...@@ -5,14 +5,6 @@ export default () => {
if (toggleContainer) { if (toggleContainer) {
const onToggleButtonClicked = isAutoSslEnabled => { 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 => { Array.from(document.querySelectorAll('.js-shown-unless-auto-ssl')).forEach(el => {
if (isAutoSslEnabled) { if (isAutoSslEnabled) {
el.classList.add('d-none'); el.classList.add('d-none');
...@@ -21,14 +13,6 @@ export default () => { ...@@ -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 => { Array.from(document.querySelectorAll('.js-enabled-unless-auto-ssl')).forEach(el => {
if (isAutoSslEnabled) { if (isAutoSslEnabled) {
el.setAttribute('disabled', 'disabled'); el.setAttribute('disabled', 'disabled');
......
...@@ -65,16 +65,14 @@ class Projects::PagesDomainsController < Projects::ApplicationController ...@@ -65,16 +65,14 @@ class Projects::PagesDomainsController < Projects::ApplicationController
private private
def create_params 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 end
def update_params 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 end
# rubocop: disable CodeReuse/ActiveRecord
def domain 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 end
# rubocop: enable CodeReuse/ActiveRecord
end end
...@@ -4,6 +4,8 @@ class PagesDomain < ApplicationRecord ...@@ -4,6 +4,8 @@ class PagesDomain < ApplicationRecord
VERIFICATION_KEY = 'gitlab-pages-verification-code'.freeze VERIFICATION_KEY = 'gitlab-pages-verification-code'.freeze
VERIFICATION_THRESHOLD = 3.days.freeze VERIFICATION_THRESHOLD = 3.days.freeze
enum certificate_source: { user_provided: 0, gitlab_provided: 1 }, _prefix: :certificate
belongs_to :project belongs_to :project
has_many :acme_orders, class_name: "PagesDomainAcmeOrder" has_many :acme_orders, class_name: "PagesDomainAcmeOrder"
...@@ -143,6 +145,34 @@ class PagesDomain < ApplicationRecord ...@@ -143,6 +145,34 @@ class PagesDomain < ApplicationRecord
self.certificate_valid_not_after = x509&.not_after self.certificate_valid_not_after = x509&.not_after
end 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 private
def set_verification_code def set_verification_code
......
...@@ -35,7 +35,7 @@ module PagesDomains ...@@ -35,7 +35,7 @@ module PagesDomains
def save_certificate(private_key, api_order) def save_certificate(private_key, api_order)
certificate = api_order.certificate 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 end
end end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
- if Gitlab.config.pages.external_https - 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_enabled = @domain.auto_ssl_enabled?
- auto_ssl_available_and_enabled = auto_ssl_available && auto_ssl_enabled - auto_ssl_available_and_enabled = auto_ssl_available && auto_ssl_enabled
...@@ -38,40 +38,24 @@ ...@@ -38,40 +38,24 @@
- docs_link_end = "</a>".html_safe - docs_link_end = "</a>".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 } = _("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) } .js-shown-unless-auto-ssl{ class: ("d-none" if auto_ssl_available_and_enabled) }
.form-group.row .form-group.row
.col-sm-2.col-form-label .col-sm-2.col-form-label
= f.label :certificate, _("Certificate (PEM)") = f.label :user_provided_certificate, _("Certificate (PEM)")
.col-sm-10 .col-sm-10
= f.text_area :certificate, = f.text_area :user_provided_certificate,
rows: 5, rows: 5,
class: "form-control js-enabled-unless-auto-ssl", class: "form-control js-enabled-unless-auto-ssl",
value: (@domain.certificate unless auto_ssl_available_and_enabled),
disabled: 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") %span.help-inline.text-muted= _("Upload a certificate for your domain with all intermediates")
.form-group.row .form-group.row
.col-sm-2.col-form-label .col-sm-2.col-form-label
= f.label :key, _("Key (PEM)") = f.label :user_provided_key, _("Key (PEM)")
.col-sm-10 .col-sm-10
= f.text_area :key, = f.text_area :user_provided_key,
rows: 5, rows: 5,
class: "form-control js-enabled-unless-auto-ssl", class: "form-control js-enabled-unless-auto-ssl",
value: (@domain.key unless auto_ssl_available_and_enabled),
disabled: auto_ssl_available_and_enabled disabled: auto_ssl_available_and_enabled
%span.help-inline.text-muted= _("Upload a private key for your certificate") %span.help-inline.text-muted= _("Upload a private key for your certificate")
......
# 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
...@@ -2333,6 +2333,7 @@ ActiveRecord::Schema.define(version: 20190619175843) do ...@@ -2333,6 +2333,7 @@ ActiveRecord::Schema.define(version: 20190619175843) do
t.boolean "auto_ssl_enabled", default: false, null: false t.boolean "auto_ssl_enabled", default: false, null: false
t.datetime_with_timezone "certificate_valid_not_before" t.datetime_with_timezone "certificate_valid_not_before"
t.datetime_with_timezone "certificate_valid_not_after" 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 ["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", "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 t.index ["project_id"], name: "index_pages_domains_on_project_id", using: :btree
......
...@@ -90,14 +90,15 @@ module API ...@@ -90,14 +90,15 @@ module API
end end
params do params do
requires :domain, type: String, desc: 'The domain' requires :domain, type: String, desc: 'The domain'
optional :certificate, allow_blank: false, types: [File, String], desc: 'The certificate' 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' optional :key, allow_blank: false, types: [File, String], desc: 'The key', as: :user_provided_key
all_or_none_of :certificate, :key all_or_none_of :user_provided_certificate, :user_provided_key
end end
post ":id/pages/domains" do post ":id/pages/domains" do
authorize! :update_pages, user_project authorize! :update_pages, user_project
pages_domain_params = declared(params, include_parent_namespaces: false) pages_domain_params = declared(params, include_parent_namespaces: false)
pages_domain = user_project.pages_domains.create(pages_domain_params) pages_domain = user_project.pages_domains.create(pages_domain_params)
if pages_domain.persisted? if pages_domain.persisted?
...@@ -110,8 +111,8 @@ module API ...@@ -110,8 +111,8 @@ module API
desc 'Updates a pages domain' desc 'Updates a pages domain'
params do params do
requires :domain, type: String, desc: 'The domain' requires :domain, type: String, desc: 'The domain'
optional :certificate, allow_blank: false, types: [File, String], desc: 'The certificate' 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' optional :key, allow_blank: false, types: [File, String], desc: 'The key', as: :user_provided_key
end end
put ":id/pages/domains/:domain", requirements: PAGES_DOMAINS_ENDPOINT_REQUIREMENTS do put ":id/pages/domains/:domain", requirements: PAGES_DOMAINS_ENDPOINT_REQUIREMENTS do
authorize! :update_pages, user_project authorize! :update_pages, user_project
...@@ -119,8 +120,8 @@ module API ...@@ -119,8 +120,8 @@ module API
pages_domain_params = declared(params, include_parent_namespaces: false) pages_domain_params = declared(params, include_parent_namespaces: false)
# Remove empty private key if certificate is not empty. # Remove empty private key if certificate is not empty.
if pages_domain_params[:certificate] && !pages_domain_params[:key] if pages_domain_params[:user_provided_certificate] && !pages_domain_params[:user_provided_key]
pages_domain_params.delete(:key) pages_domain_params.delete(:user_provided_key)
end end
if pages_domain.update(pages_domain_params) if pages_domain.update(pages_domain_params)
......
...@@ -9942,9 +9942,6 @@ msgstr "" ...@@ -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." 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 "" 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." 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 "" msgstr ""
...@@ -10278,9 +10275,6 @@ msgstr "" ...@@ -10278,9 +10275,6 @@ msgstr ""
msgid "This branch has changed since you started editing. Would you like to create a new branch?" msgid "This branch has changed since you started editing. Would you like to create a new branch?"
msgstr "" 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." 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 "" msgstr ""
......
...@@ -15,7 +15,10 @@ describe Projects::PagesDomainsController do ...@@ -15,7 +15,10 @@ describe Projects::PagesDomainsController do
end end
let(:pages_domain_params) do 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 end
before do before do
...@@ -84,21 +87,22 @@ describe Projects::PagesDomainsController do ...@@ -84,21 +87,22 @@ describe Projects::PagesDomainsController do
controller.instance_variable_set(:@domain, pages_domain) controller.instance_variable_set(:@domain, pages_domain)
end end
let(:pages_domain_params) do
attributes_for(:pages_domain).slice(:key, :certificate)
end
let(:params) do let(:params) do
request_params.merge(id: pages_domain.domain, pages_domain: pages_domain_params) request_params.merge(id: pages_domain.domain, pages_domain: pages_domain_params)
end end
it 'updates the domain' do context 'with valid params' do
expect(pages_domain) let(:pages_domain_params) do
.to receive(:update) attributes_for(:pages_domain, :with_trusted_chain).slice(:key, :certificate).tap do |params|
.with(ActionController::Parameters.new(pages_domain_params).permit!) params[:user_provided_key] = params.delete(:key)
.and_return(true) params[:user_provided_certificate] = params.delete(:certificate)
end
end
it 'updates the domain' do
expect do
patch(:update, params: params) patch(:update, params: params)
end.to change { pages_domain.reload.certificate }.to(pages_domain_params[:user_provided_certificate])
end end
it 'redirects to the project page' do it 'redirects to the project page' do
...@@ -107,25 +111,35 @@ describe Projects::PagesDomainsController do ...@@ -107,25 +111,35 @@ describe Projects::PagesDomainsController do
expect(flash[:notice]).to eq 'Domain was updated' expect(flash[:notice]).to eq 'Domain was updated'
expect(response).to redirect_to(project_pages_path(project)) expect(response).to redirect_to(project_pages_path(project))
end end
end
context 'with key parameter' do
before do
pages_domain.update!(key: nil, certificate: nil, certificate_source: 'gitlab_provided')
end
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 context 'the domain is invalid' do
it 'renders the edit action' do let(:pages_domain_params) { { user_provided_certificate: 'blabla' } }
allow(pages_domain).to receive(:update).and_return(false)
it 'renders the edit action' do
patch(:update, params: params) patch(:update, params: params)
expect(response).to render_template('edit') expect(response).to render_template('edit')
end end
end end
context 'the parameters include the domain' do context 'when parameters include the domain' do
it 'renders 400 Bad Request' do it 'does not update domain' do
expect(pages_domain) expect do
.to receive(:update)
.with(hash_not_including(:domain))
.and_return(true)
patch(:update, params: params.deep_merge(pages_domain: { domain: 'abc' })) patch(:update, params: params.deep_merge(pages_domain: { domain: 'abc' }))
end.not_to change { pages_domain.reload.domain }
end end
end end
end end
......
...@@ -180,5 +180,9 @@ Iy6oRpHaCF/2obZdIdgf9rlyz0fkqyHJc9GkioSoOhJZxEV2SgAkap8yS0sX2tJ9 ...@@ -180,5 +180,9 @@ Iy6oRpHaCF/2obZdIdgf9rlyz0fkqyHJc9GkioSoOhJZxEV2SgAkap8yS0sX2tJ9
ZDXgrA== ZDXgrA==
-----END CERTIFICATE-----' -----END CERTIFICATE-----'
end end
trait :letsencrypt do
certificate_source { :gitlab_provided }
end
end end
end end
...@@ -2,62 +2,25 @@ ...@@ -2,62 +2,25 @@
require 'spec_helper' require 'spec_helper'
describe "Pages with Let's Encrypt", :https_pages_enabled do describe "Pages with Let's Encrypt", :https_pages_enabled do
include LetsEncryptHelpers
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:role) { :maintainer } let(:role) { :maintainer }
let(:certificate_pem) do let(:certificate_pem) { attributes_for(:pages_domain)[:certificate] }
<<~PEM
-----BEGIN CERTIFICATE----- let(:certificate_key) { attributes_for(:pages_domain)[:key] }
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_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
before do before do
allow(Gitlab.config.pages).to receive(:enabled).and_return(true) allow(Gitlab.config.pages).to receive(:enabled).and_return(true)
stub_lets_encrypt_settings
project.add_role(user, role) project.add_role(user, role)
sign_in(user) sign_in(user)
project.namespace.update(owner: user) project.namespace.update(owner: user)
allow_any_instance_of(Project).to receive(:pages_deployed?) { true } allow_any_instance_of(Project).to receive(:pages_deployed?) { true }
end end
context 'when the page_auto_ssl feature flag is enabled' do
before do
stub_feature_flags(pages_auto_ssl: true)
end
context 'when the auto SSL management is initially disabled' do context 'when the auto SSL management is initially disabled' do
let(:domain) do let(:domain) do
create(:pages_domain, auto_ssl_enabled: false, project: project) create(:pages_domain, auto_ssl_enabled: false, project: project)
...@@ -77,7 +40,6 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do ...@@ -77,7 +40,6 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do
expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'true' 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 'Certificate (PEM)', type: 'textarea'
expect(page).not_to have_field 'Key (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'
...@@ -87,14 +49,14 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do ...@@ -87,14 +49,14 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do
context 'when the auto SSL management is initially enabled' do context 'when the auto SSL management is initially enabled' do
let(:domain) do let(:domain) do
create(:pages_domain, auto_ssl_enabled: true, project: project) create(:pages_domain, :letsencrypt, auto_ssl_enabled: true, project: project)
end end
it 'disables auto SSL and dynamically updates the form accordingly', :js do it 'disables auto SSL and dynamically updates the form accordingly', :js do
visit edit_project_pages_domain_path(project, domain) visit edit_project_pages_domain_path(project, domain)
expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'true' 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 'Certificate (PEM)', type: 'textarea'
expect(page).not_to have_field 'Key (PEM)', type: 'textarea' 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
...@@ -111,15 +73,48 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do ...@@ -111,15 +73,48 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do
expect(domain.reload.auto_ssl_enabled).to eq false expect(domain.reload.auto_ssl_enabled).to eq false
end end
end end
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 end
context 'when the page_auto_ssl feature flag is disabled' do 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
include_examples 'user sees private keys only for user provided certificate'
context 'when letsencrypt is disabled' do
let(:domain) do let(:domain) do
create(:pages_domain, auto_ssl_enabled: false, project: project) create(:pages_domain, auto_ssl_enabled: false, project: project)
end end
before do 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) visit edit_project_pages_domain_path(project, domain)
end end
...@@ -127,5 +122,7 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do ...@@ -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 it "does not render the Let's Encrypt field", :js do
expect(page).not_to have_selector '.js-auto-ssl-toggle-container' expect(page).not_to have_selector '.js-auto-ssl-toggle-container'
end end
include_examples 'user sees private keys only for user provided certificate'
end end
end end
# frozen_string_literal: true # frozen_string_literal: true
require 'spec_helper' require 'spec_helper'
shared_examples 'pages domain editing' do shared_examples 'pages settings editing' do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:role) { :maintainer } let(:role) { :maintainer }
...@@ -321,19 +321,15 @@ shared_examples 'pages domain editing' do ...@@ -321,19 +321,15 @@ shared_examples 'pages domain editing' do
end end
describe 'Pages' do describe 'Pages' do
context 'when pages_auto_ssl feature flag is disabled' do include LetsEncryptHelpers
before do
stub_feature_flags(pages_auto_ssl: false)
end
include_examples 'pages domain editing' include_examples 'pages settings editing'
end
context 'when pages_auto_ssl feature flag is enabled' do context 'when letsencrypt support is enabled' do
before do before do
stub_feature_flags(pages_auto_ssl: true) stub_lets_encrypt_settings
end end
include_examples 'pages domain editing' include_examples 'pages settings editing'
end end
end end
...@@ -356,6 +356,102 @@ describe PagesDomain do ...@@ -356,6 +356,102 @@ describe PagesDomain do
end end
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 describe '.for_removal' do
subject { described_class.for_removal } subject { described_class.for_removal }
......
...@@ -359,6 +359,14 @@ describe API::PagesDomains do ...@@ -359,6 +359,14 @@ describe API::PagesDomains do
expect(pages_domain_secure.certificate).to eq(params_secure_nokey[:certificate]) expect(pages_domain_secure.certificate).to eq(params_secure_nokey[:certificate])
end 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 it 'fails to update pages domain adding certificate without key' do
put api(route_domain, user), params: params_secure_nokey put api(route_domain, user), params: params_secure_nokey
......
...@@ -137,6 +137,12 @@ describe PagesDomains::ObtainLetsEncryptCertificateService do ...@@ -137,6 +137,12 @@ describe PagesDomains::ObtainLetsEncryptCertificateService do
expect(pages_domain.certificate).to eq(certificate) expect(pages_domain.certificate).to eq(certificate)
end 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 it 'removes order from database' do
service.execute service.execute
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment