From cbb2efa94938c138b074715187c9df230ce4828e Mon Sep 17 00:00:00 2001 From: Rajendra Kadam <rkadam@gitlab.com> Date: Mon, 22 Jan 2024 18:36:14 +0000 Subject: [PATCH] Update usage of UrlBlocker to HTTP_V2 in services Update specs to confirm the change Prepend colons for call in EE to CurrentSettings --- ee/app/models/ee/application_setting.rb | 5 +++-- lib/bulk_imports/common/pipelines/wiki_pipeline.rb | 7 ++++++- .../projects/pipelines/repository_pipeline.rb | 7 ++++++- .../pipelines/snippets_repository_pipeline.rb | 3 ++- .../error_repository/open_api_strategy.rb | 6 +++++- .../github_gists_import/importer/gist_importer.rb | 12 +++++++----- lib/gitlab/kubernetes/kube_client.rb | 6 +++++- lib/gitlab/octokit/middleware.rb | 5 +++-- .../importer/gist_importer_spec.rb | 14 ++++++++++---- 9 files changed, 47 insertions(+), 18 deletions(-) diff --git a/ee/app/models/ee/application_setting.rb b/ee/app/models/ee/application_setting.rb index 947b1fe2a8641..fc0d7123f7e40 100644 --- a/ee/app/models/ee/application_setting.rb +++ b/ee/app/models/ee/application_setting.rb @@ -545,10 +545,11 @@ def check_elasticsearch_url_scheme # ElasticSearch only exposes a RESTful API, hence we need # to use the HTTP protocol on all URLs. elasticsearch_url.each do |str| - ::Gitlab::UrlBlocker.validate!(str, + ::Gitlab::HTTP_V2::UrlBlocker.validate!(str, schemes: %w[http https], allow_localhost: true, - dns_rebind_protection: false) + dns_rebind_protection: false, + deny_all_requests_except_allowed: deny_all_requests_except_allowed?) end rescue ::Gitlab::HTTP_V2::UrlBlocker::BlockedUrlError errors.add(:elasticsearch_url, "only supports valid HTTP(S) URLs.") diff --git a/lib/bulk_imports/common/pipelines/wiki_pipeline.rb b/lib/bulk_imports/common/pipelines/wiki_pipeline.rb index 68d511b065fc8..429a28dcb4c8e 100644 --- a/lib/bulk_imports/common/pipelines/wiki_pipeline.rb +++ b/lib/bulk_imports/common/pipelines/wiki_pipeline.rb @@ -22,7 +22,12 @@ def load(context, data) wiki = context.portable.wiki url = data[:url].sub("://", "://oauth2:#{context.configuration.access_token}@") - Gitlab::UrlBlocker.validate!(url, schemes: %w[http https], allow_local_network: allow_local_requests?, allow_localhost: allow_local_requests?) + Gitlab::HTTP_V2::UrlBlocker.validate!( + url, + schemes: %w[http https], + allow_local_network: allow_local_requests?, + allow_localhost: allow_local_requests?, + deny_all_requests_except_allowed: Gitlab::CurrentSettings.deny_all_requests_except_allowed?) wiki.create_wiki_repository wiki.repository.fetch_as_mirror(url) diff --git a/lib/bulk_imports/projects/pipelines/repository_pipeline.rb b/lib/bulk_imports/projects/pipelines/repository_pipeline.rb index a2b1f8c51768e..04c887441f48b 100644 --- a/lib/bulk_imports/projects/pipelines/repository_pipeline.rb +++ b/lib/bulk_imports/projects/pipelines/repository_pipeline.rb @@ -21,7 +21,12 @@ def load(context, data) url = url.sub("://", "://oauth2:#{context.configuration.access_token}@") project = context.portable - Gitlab::UrlBlocker.validate!(url, schemes: %w[http https], allow_local_network: allow_local_requests?, allow_localhost: allow_local_requests?) + Gitlab::HTTP_V2::UrlBlocker.validate!( + url, + schemes: %w[http https], + allow_local_network: allow_local_requests?, + allow_localhost: allow_local_requests?, + deny_all_requests_except_allowed: Gitlab::CurrentSettings.deny_all_requests_except_allowed?) project.ensure_repository project.repository.fetch_as_mirror(url) diff --git a/lib/bulk_imports/projects/pipelines/snippets_repository_pipeline.rb b/lib/bulk_imports/projects/pipelines/snippets_repository_pipeline.rb index 39c9c121797df..a371c33d9ea66 100644 --- a/lib/bulk_imports/projects/pipelines/snippets_repository_pipeline.rb +++ b/lib/bulk_imports/projects/pipelines/snippets_repository_pipeline.rb @@ -53,10 +53,11 @@ def oauth2(url) end def validate_url(url) - Gitlab::UrlBlocker.validate!( + Gitlab::HTTP_V2::UrlBlocker.validate!( url, allow_local_network: allow_local_requests?, allow_localhost: allow_local_requests?, + deny_all_requests_except_allowed: Gitlab::CurrentSettings.deny_all_requests_except_allowed?, schemes: %w[http https] ) end diff --git a/lib/gitlab/error_tracking/error_repository/open_api_strategy.rb b/lib/gitlab/error_tracking/error_repository/open_api_strategy.rb index 3b0b4c6e9351e..9b9b0f65633b7 100644 --- a/lib/gitlab/error_tracking/error_repository/open_api_strategy.rb +++ b/lib/gitlab/error_tracking/error_repository/open_api_strategy.rb @@ -232,7 +232,11 @@ def configured_api_url url = Gitlab::CurrentSettings.current_application_settings.error_tracking_api_url || 'http://localhost:8080' - Gitlab::UrlBlocker.validate!(url, schemes: %w[http https], allow_localhost: true) + Gitlab::HTTP_V2::UrlBlocker.validate!( + url, + schemes: %w[http https], + allow_localhost: true, + deny_all_requests_except_allowed: Gitlab::CurrentSettings.deny_all_requests_except_allowed?) URI(url) end diff --git a/lib/gitlab/github_gists_import/importer/gist_importer.rb b/lib/gitlab/github_gists_import/importer/gist_importer.rb index 71dfe5e2aa5b2..c2456f8371110 100644 --- a/lib/gitlab/github_gists_import/importer/gist_importer.rb +++ b/lib/gitlab/github_gists_import/importer/gist_importer.rb @@ -58,11 +58,13 @@ def import_repository end def get_resolved_address - validated_pull_url, host = Gitlab::UrlBlocker.validate!(gist.git_pull_url, - schemes: Project::VALID_IMPORT_PROTOCOLS, - ports: Project::VALID_IMPORT_PORTS, - allow_localhost: allow_local_requests?, - allow_local_network: allow_local_requests?) + validated_pull_url, host = Gitlab::HTTP_V2::UrlBlocker.validate!( + gist.git_pull_url, + schemes: Project::VALID_IMPORT_PROTOCOLS, + ports: Project::VALID_IMPORT_PORTS, + allow_localhost: allow_local_requests?, + allow_local_network: allow_local_requests?, + deny_all_requests_except_allowed: Gitlab::CurrentSettings.deny_all_requests_except_allowed?) host.present? ? validated_pull_url.host.to_s : '' end diff --git a/lib/gitlab/kubernetes/kube_client.rb b/lib/gitlab/kubernetes/kube_client.rb index 44e53e9ec70d9..593090902e796 100644 --- a/lib/gitlab/kubernetes/kube_client.rb +++ b/lib/gitlab/kubernetes/kube_client.rb @@ -161,7 +161,11 @@ def create_or_update_secret(resource) def validate_url! return if Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? - Gitlab::UrlBlocker.validate!(api_prefix, allow_local_network: false, schemes: %w[http https]) + Gitlab::HTTP_V2::UrlBlocker.validate!( + api_prefix, + allow_local_network: false, + schemes: %w[http https], + deny_all_requests_except_allowed: Gitlab::CurrentSettings.deny_all_requests_except_allowed?) end def service_account_exists?(resource) diff --git a/lib/gitlab/octokit/middleware.rb b/lib/gitlab/octokit/middleware.rb index f944f9827a328..a93526da5ca20 100644 --- a/lib/gitlab/octokit/middleware.rb +++ b/lib/gitlab/octokit/middleware.rb @@ -8,11 +8,12 @@ def initialize(app) end def call(env) - Gitlab::UrlBlocker.validate!(env[:url], + Gitlab::HTTP_V2::UrlBlocker.validate!(env[:url], schemes: %w[http https], allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests?, - dns_rebind_protection: dns_rebind_protection? + dns_rebind_protection: dns_rebind_protection?, + deny_all_requests_except_allowed: Gitlab::CurrentSettings.deny_all_requests_except_allowed? ) @app.call(env) diff --git a/spec/lib/gitlab/github_gists_import/importer/gist_importer_spec.rb b/spec/lib/gitlab/github_gists_import/importer/gist_importer_spec.rb index b098a1516601f..b64348d447baf 100644 --- a/spec/lib/gitlab/github_gists_import/importer/gist_importer_spec.rb +++ b/spec/lib/gitlab/github_gists_import/importer/gist_importer_spec.rb @@ -167,13 +167,16 @@ before do allow(::Gitlab::CurrentSettings) .to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(true) + allow(::Gitlab::CurrentSettings) + .to receive(:deny_all_requests_except_allowed?).and_return(true) end it 'raises error' do - expect(Gitlab::UrlBlocker) + expect(Gitlab::HTTP_V2::UrlBlocker) .to receive(:validate!) .with(url, ports: [80, 443], schemes: %w[http https git], - allow_localhost: true, allow_local_network: true) + allow_localhost: true, allow_local_network: true, + deny_all_requests_except_allowed: true) .and_raise(Gitlab::HTTP_V2::UrlBlocker::BlockedUrlError) expect { subject.execute }.to raise_error(Gitlab::HTTP_V2::UrlBlocker::BlockedUrlError) @@ -184,13 +187,16 @@ before do allow(::Gitlab::CurrentSettings) .to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(false) + allow(::Gitlab::CurrentSettings) + .to receive(:deny_all_requests_except_allowed?).and_return(true) end it 'raises error' do - expect(Gitlab::UrlBlocker) + expect(Gitlab::HTTP_V2::UrlBlocker) .to receive(:validate!) .with(url, ports: [80, 443], schemes: %w[http https git], - allow_localhost: false, allow_local_network: false) + allow_localhost: false, allow_local_network: false, + deny_all_requests_except_allowed: true) .and_raise(Gitlab::HTTP_V2::UrlBlocker::BlockedUrlError) expect { subject.execute }.to raise_error(Gitlab::HTTP_V2::UrlBlocker::BlockedUrlError) -- GitLab