From 554bb1eb1f1aa5aa45b5e4a1ee1557da0ccc59c0 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sat, 16 Dec 2023 09:10:45 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- ...obtain_lets_encrypt_certificate_service.rb | 18 ++++++++++++++ lib/gitlab/pages/url_builder.rb | 18 ++++++++++---- spec/lib/gitlab/pages/url_builder_spec.rb | 8 ++++++- spec/models/pages/lookup_path_spec.rb | 10 ++++++++ ...n_lets_encrypt_certificate_service_spec.rb | 24 +++++++++++++++++++ 5 files changed, 72 insertions(+), 6 deletions(-) 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 1733021cbb5..c11b019cee5 100644 --- a/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb +++ b/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb @@ -10,6 +10,9 @@ module PagesDomains # no particular SLA, usually takes 10-15 seconds CERTIFICATE_PROCESSING_DELAY = 1.minute.freeze + # Maximum domain length for Let's Encrypt + MAX_DOMAIN_LENGTH = 64 + attr_reader :pages_domain def initialize(pages_domain) @@ -17,6 +20,11 @@ module PagesDomains end def execute + if pages_domain.domain.bytesize > MAX_DOMAIN_LENGTH + log_domain_length_error + return + end + pages_domain.acme_orders.expired.delete_all acme_order = pages_domain.acme_orders.first @@ -59,6 +67,16 @@ module PagesDomains NotificationService.new.pages_domain_auto_ssl_failed(pages_domain) end + def log_domain_length_error + Gitlab::AppLogger.error( + message: "Domain name too long for Let's Encrypt certificate", + pages_domain: pages_domain.domain, + pages_domain_bytesize: pages_domain.domain.bytesize, + max_allowed_bytesize: MAX_DOMAIN_LENGTH, + project_id: pages_domain.project_id + ) + end + def log_error(api_order) Gitlab::AppLogger.error( message: "Failed to obtain Let's Encrypt certificate", diff --git a/lib/gitlab/pages/url_builder.rb b/lib/gitlab/pages/url_builder.rb index 7fe9c611f5b..f01ec54b853 100644 --- a/lib/gitlab/pages/url_builder.rb +++ b/lib/gitlab/pages/url_builder.rb @@ -14,10 +14,9 @@ module Gitlab end def pages_url(with_unique_domain: false) + return namespace_in_path_url(with_unique_domain && unique_domain_enabled?) if config.namespace_in_path return unique_url if with_unique_domain && unique_domain_enabled? - return "#{pages_base_url}/#{project_namespace}/#{project_path}".downcase if config.namespace_in_path - project_path_url = "#{config.protocol}://#{project_path}".downcase # If the project path is the same as host, we serve it as group page @@ -31,6 +30,7 @@ module Gitlab def unique_host return unless unique_domain_enabled? + return if config.namespace_in_path URI(unique_url).host end @@ -73,9 +73,17 @@ module Gitlab def pages_base_url @pages_url ||= URI(config.url) - .tap { |url| url.port = config.port } - .to_s - .downcase + .tap { |url| url.port = config.port } + .to_s + .downcase + end + + def namespace_in_path_url(with_unique_domain) + if with_unique_domain + "#{pages_base_url}/#{project.project_setting.pages_unique_domain}".downcase + else + "#{pages_base_url}/#{project_namespace}/#{project_path}".downcase + end end def url_for(subdomain) diff --git a/spec/lib/gitlab/pages/url_builder_spec.rb b/spec/lib/gitlab/pages/url_builder_spec.rb index 9b6ecffa426..1a97ca01c3e 100644 --- a/spec/lib/gitlab/pages/url_builder_spec.rb +++ b/spec/lib/gitlab/pages/url_builder_spec.rb @@ -177,7 +177,7 @@ RSpec.describe Gitlab::Pages::UrlBuilder, feature_category: :pages do context 'when pages_unique_domain_enabled is true' do let(:unique_domain_enabled) { true } - it { is_expected.to eq('http://unique-domain.example.com') } + it { is_expected.to eq('http://example.com/unique-domain') } end end end @@ -192,6 +192,12 @@ RSpec.describe Gitlab::Pages::UrlBuilder, feature_category: :pages do it { is_expected.to be_nil } end + context 'when namespace_in_path is true' do + let(:namespace_in_path) { true } + + it { is_expected.to be_nil } + end + context 'when pages_unique_domain_enabled is true' do let(:unique_domain_enabled) { true } diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index aa92a5d028c..ccca6e7a48a 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -134,6 +134,16 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end end + context 'when namespace_in_path is enabled' do + before do + stub_pages_setting(namespace_in_path: true) + end + + it 'returns nil' do + expect(lookup_path.unique_host).to be_nil + end + end + context 'when unique domain is enabled' do it 'returns the project unique domain' do project.project_setting.pages_unique_domain_enabled = true 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 0e46391c0ad..63b5d54a18d 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 @@ -188,4 +188,28 @@ RSpec.describe PagesDomains::ObtainLetsEncryptCertificateService, feature_catego service.execute end end + + context 'when the domain URL is longer than 64 characters' do + let(:long_domain) { "a.b.c.#{'d' * 63}" } + let(:pages_domain) { create(:pages_domain, :without_certificate, :without_key, domain: long_domain) } + let(:service) { described_class.new(pages_domain) } + + it 'logs an error and does not proceed with certificate acquisition' do + expect(Gitlab::AppLogger).to receive(:error).with( + hash_including( + message: "Domain name too long for Let's Encrypt certificate", + pages_domain: long_domain, + pages_domain_bytesize: long_domain.bytesize, + max_allowed_bytesize: described_class::MAX_DOMAIN_LENGTH, + project_id: pages_domain.project_id + ) + ) + + # Ensure that the certificate acquisition is not attempted + expect(::PagesDomains::CreateAcmeOrderService).not_to receive(:new) + expect(PagesDomainSslRenewalWorker).not_to receive(:perform_in) + + service.execute + end + end end