summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/ci/build_runner_session.rb20
-rw-r--r--app/models/project.rb2
-rw-r--r--app/models/user.rb2
-rw-r--r--app/policies/packages/policies/group_policy.rb2
-rw-r--r--lib/gitlab/hook_data/project_builder.rb2
-rw-r--r--lib/gitlab/hook_data/project_member_builder.rb2
-rw-r--r--spec/lib/gitlab/hook_data/project_builder_spec.rb4
-rw-r--r--spec/lib/gitlab/hook_data/project_member_builder_spec.rb2
-rw-r--r--spec/models/ci/build_runner_session_spec.rb51
-rw-r--r--spec/models/project_spec.rb4
-rw-r--r--spec/requests/api/ci/runner/jobs_request_post_spec.rb35
11 files changed, 108 insertions, 18 deletions
diff --git a/app/models/ci/build_runner_session.rb b/app/models/ci/build_runner_session.rb
index c6dbb5d0a43..0f37ce70964 100644
--- a/app/models/ci/build_runner_session.rb
+++ b/app/models/ci/build_runner_session.rb
@@ -13,14 +13,15 @@ module Ci
belongs_to :build, class_name: 'Ci::Build', inverse_of: :runner_session
validates :build, presence: true
- validates :url, addressable_url: { schemes: %w(https) }
+ validates :url, public_url: { schemes: %w(https) }
def terminal_specification
- wss_url = Gitlab::UrlHelpers.as_wss(self.url)
+ wss_url = Gitlab::UrlHelpers.as_wss(Addressable::URI.escape(self.url))
return {} unless wss_url.present?
- wss_url = "#{wss_url}/exec"
- channel_specification(wss_url, TERMINAL_SUBPROTOCOL)
+ parsed_wss_url = URI.parse(wss_url)
+ parsed_wss_url.path += '/exec'
+ channel_specification(parsed_wss_url, TERMINAL_SUBPROTOCOL)
end
def service_specification(service: nil, path: nil, port: nil, subprotocols: nil)
@@ -28,20 +29,21 @@ module Ci
port = port.presence || DEFAULT_PORT_NAME
service = service.presence || DEFAULT_SERVICE_NAME
- url = "#{self.url}/proxy/#{service}/#{port}/#{path}"
+ parsed_url = URI.parse(Addressable::URI.escape(self.url))
+ parsed_url.path += "/proxy/#{service}/#{port}/#{path}"
subprotocols = subprotocols.presence || ::Ci::BuildRunnerSession::TERMINAL_SUBPROTOCOL
- channel_specification(url, subprotocols)
+ channel_specification(parsed_url, subprotocols)
end
private
- def channel_specification(url, subprotocol)
- return {} if subprotocol.blank? || url.blank?
+ def channel_specification(parsed_url, subprotocol)
+ return {} if subprotocol.blank? || parsed_url.blank?
{
subprotocols: Array(subprotocol),
- url: url,
+ url: Addressable::URI.unescape(parsed_url.to_s),
headers: { Authorization: [authorization.presence] }.compact,
ca_pem: certificate.presence
}
diff --git a/app/models/project.rb b/app/models/project.rb
index a07d4147228..0c4f76fb2b9 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -2152,8 +2152,8 @@ class Project < ApplicationRecord
end
def after_import
- repository.remove_prohibited_branches
repository.expire_content_cache
+ repository.remove_prohibited_branches
wiki.repository.expire_content_cache
DetectRepositoryLanguagesWorker.perform_async(id)
diff --git a/app/models/user.rb b/app/models/user.rb
index 24f947183a2..b4b8a7ef7ad 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1556,7 +1556,7 @@ class User < ApplicationRecord
name: name,
username: username,
avatar_url: avatar_url(only_path: false),
- email: public_email.presence || _('[REDACTED]')
+ email: webhook_email
}
end
diff --git a/app/policies/packages/policies/group_policy.rb b/app/policies/packages/policies/group_policy.rb
index 32dbcb1b65b..d8c20c7a90a 100644
--- a/app/policies/packages/policies/group_policy.rb
+++ b/app/policies/packages/policies/group_policy.rb
@@ -25,3 +25,5 @@ module Packages
end
end
end
+
+Packages::Policies::GroupPolicy.prepend_mod_with('Packages::Policies::GroupPolicy')
diff --git a/lib/gitlab/hook_data/project_builder.rb b/lib/gitlab/hook_data/project_builder.rb
index ebd97d3ab1b..aec842e061f 100644
--- a/lib/gitlab/hook_data/project_builder.rb
+++ b/lib/gitlab/hook_data/project_builder.rb
@@ -57,7 +57,7 @@ module Gitlab
end
def user_email(user)
- user.respond_to?(:email) ? user.email : ""
+ user.respond_to?(:webhook_email) ? user.webhook_email : ""
end
def event_specific_project_data(event)
diff --git a/lib/gitlab/hook_data/project_member_builder.rb b/lib/gitlab/hook_data/project_member_builder.rb
index 2ee61705ec1..be5bde356de 100644
--- a/lib/gitlab/hook_data/project_member_builder.rb
+++ b/lib/gitlab/hook_data/project_member_builder.rb
@@ -43,7 +43,7 @@ module Gitlab
project_id: project.id,
user_username: project_member.user.username,
user_name: project_member.user.name,
- user_email: project_member.user.email,
+ user_email: project_member.user.webhook_email,
user_id: project_member.user.id,
access_level: project_member.human_access,
project_visibility: project.visibility
diff --git a/spec/lib/gitlab/hook_data/project_builder_spec.rb b/spec/lib/gitlab/hook_data/project_builder_spec.rb
index 729712510ea..f80faac563d 100644
--- a/spec/lib/gitlab/hook_data/project_builder_spec.rb
+++ b/spec/lib/gitlab/hook_data/project_builder_spec.rb
@@ -29,8 +29,8 @@ RSpec.describe Gitlab::HookData::ProjectBuilder do
expect(data[:path_with_namespace]).to eq(project.full_path)
expect(data[:project_id]).to eq(project.id)
expect(data[:owner_name]).to eq('John')
- expect(data[:owner_email]).to eq('john@example.com')
- expect(data[:owners]).to contain_exactly({ name: 'John', email: 'john@example.com' })
+ expect(data[:owner_email]).to eq(_('[REDACTED]'))
+ expect(data[:owners]).to contain_exactly({ name: 'John', email: _('[REDACTED]') })
expect(data[:project_visibility]).to eq('internal')
end
end
diff --git a/spec/lib/gitlab/hook_data/project_member_builder_spec.rb b/spec/lib/gitlab/hook_data/project_member_builder_spec.rb
index 76446adf7b7..ea71c5442f4 100644
--- a/spec/lib/gitlab/hook_data/project_member_builder_spec.rb
+++ b/spec/lib/gitlab/hook_data/project_member_builder_spec.rb
@@ -27,7 +27,7 @@ RSpec.describe Gitlab::HookData::ProjectMemberBuilder do
expect(data[:user_username]).to eq('johndoe')
expect(data[:user_name]).to eq('John Doe')
expect(data[:user_id]).to eq(user.id)
- expect(data[:user_email]).to eq('john@example.com')
+ expect(data[:user_email]).to eq(_('[REDACTED]'))
expect(data[:access_level]).to eq('Developer')
expect(data[:project_visibility]).to eq('internal')
end
diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb
index 9bb8a1bd626..8dfe854511c 100644
--- a/spec/models/ci/build_runner_session_spec.rb
+++ b/spec/models/ci/build_runner_session_spec.rb
@@ -13,6 +13,45 @@ RSpec.describe Ci::BuildRunnerSession, model: true do
it { is_expected.to validate_presence_of(:build) }
it { is_expected.to validate_presence_of(:url).with_message('must be a valid URL') }
+ context 'url validation of local web hook address' do
+ let(:url) { 'https://127.0.0.1:7777' }
+
+ subject(:build_with_local_runner_session_url) do
+ create(:ci_build).tap { |b| b.update!(runner_session_attributes: { url: url }) }
+ end
+
+ context 'with allow_local_requests_from_web_hooks_and_services? stubbed' do
+ before do
+ allow(ApplicationSetting).to receive(:current).and_return(ApplicationSetting.new)
+ stub_application_setting(allow_local_requests_from_web_hooks_and_services: allow_local_requests)
+ end
+
+ context 'as returning true' do
+ let(:allow_local_requests) { true }
+
+ it 'creates a new session', :aggregate_failures do
+ session = build_with_local_runner_session_url.reload.runner_session
+
+ expect(session.errors).to be_empty
+ expect(session).to be_a(Ci::BuildRunnerSession)
+ expect(session.url).to eq(url)
+ end
+ end
+
+ context 'as returning false' do
+ let(:allow_local_requests) { false }
+
+ it 'does not create a new session' do
+ expect { build_with_local_runner_session_url }.to raise_error(ActiveRecord::RecordInvalid) do |err|
+ expect(err.record.errors.full_messages).to include(
+ 'Runner session url is blocked: Requests to localhost are not allowed'
+ )
+ end
+ end
+ end
+ end
+ end
+
context 'nested attribute assignment' do
it 'creates a new session' do
simple_build = create(:ci_build)
@@ -49,6 +88,12 @@ RSpec.describe Ci::BuildRunnerSession, model: true do
expect(specification).to be_empty
end
+ it 'returns url with appended query if url has query' do
+ subject.url = 'https://new.example.com:7777/some_path?dummy='
+
+ expect(specification[:url]).to eq('wss://new.example.com:7777/some_path/exec?dummy=')
+ end
+
context 'when url is present' do
it 'returns ca_pem nil if empty certificate' do
subject.certificate = ''
@@ -85,6 +130,12 @@ RSpec.describe Ci::BuildRunnerSession, model: true do
expect(specification).to be_empty
end
+ it 'returns url with appended query if url has query' do
+ subject.url = 'https://new.example.com:7777/some_path?dummy='
+
+ expect(specification[:url]).to eq("https://new.example.com:7777/some_path/proxy/#{service}/#{port}/#{path}?dummy=")
+ end
+
context 'when port is not present' do
let(:port) { nil }
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 8cccc9ad83e..1cae03ae2ae 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -5512,8 +5512,8 @@ RSpec.describe Project, factory_default: :keep do
let(:import_state) { create(:import_state, project: project) }
it 'runs the correct hooks' do
- expect(project.repository).to receive(:remove_prohibited_branches)
- expect(project.repository).to receive(:expire_content_cache)
+ expect(project.repository).to receive(:expire_content_cache).ordered
+ expect(project.repository).to receive(:remove_prohibited_branches).ordered
expect(project.wiki.repository).to receive(:expire_content_cache)
expect(import_state).to receive(:finish)
expect(project).to receive(:update_project_counter_caches)
diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb
index 1cb4cc93ea5..d69a3f5a980 100644
--- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb
+++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb
@@ -949,6 +949,41 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
end
end
+ context 'with session url set to local URL' do
+ let(:job_params) { { session: { url: 'https://127.0.0.1:7777' } } }
+
+ context 'with allow_local_requests_from_web_hooks_and_services? stubbed' do
+ before do
+ allow(ApplicationSetting).to receive(:current).and_return(ApplicationSetting.new)
+ stub_application_setting(allow_local_requests_from_web_hooks_and_services: allow_local_requests)
+ ci_build
+ end
+
+ let(:ci_build) { create(:ci_build, :pending, :queued, pipeline: pipeline) }
+
+ context 'as returning true' do
+ let(:allow_local_requests) { true }
+
+ it 'creates a new session' do
+ request_job(**job_params)
+
+ expect(response).to have_gitlab_http_status(:created)
+ end
+ end
+
+ context 'as returning false' do
+ let(:allow_local_requests) { false }
+
+ it 'returns :unprocessable_entity status code', :aggregate_failures do
+ request_job(**job_params)
+
+ expect(response).to have_gitlab_http_status(:conflict)
+ expect(response.body).to include('409 Conflict')
+ end
+ end
+ end
+ end
+
def request_job(token = runner.token, **params)
new_params = params.merge(token: token, last_update: last_update)
post api('/jobs/request'), params: new_params.to_json, headers: { 'User-Agent' => user_agent, 'Content-Type': 'application/json' }
View Lorry Controller Status