Properly handle multiple X-Forwarded-For addresses in runner IP
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24624 extracted the X-Forwarded-For address directly, but this didn't consider the case where multiple proxies are in the chain. To fix this, we use the Rails implementation to filter trusted proxies, as documented by Grape: https://github.com/ruby-grape/grape#remote-ip Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/58103
This commit is contained in:
parent
c44c83c447
commit
d03b7bb1e0
3 changed files with 15 additions and 1 deletions
5
changelogs/unreleased/sh-fix-issue-58103.yml
Normal file
5
changelogs/unreleased/sh-fix-issue-58103.yml
Normal file
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Properly handle multiple X-Forwarded-For addresses in runner IP
|
||||||
|
merge_request: 25511
|
||||||
|
author:
|
||||||
|
type: fixed
|
|
@ -26,7 +26,7 @@ module API
|
||||||
end
|
end
|
||||||
|
|
||||||
def get_runner_ip
|
def get_runner_ip
|
||||||
{ ip_address: request.env["HTTP_X_FORWARDED_FOR"] || request.ip }
|
{ ip_address: env["action_dispatch.remote_ip"].to_s || request.ip }
|
||||||
end
|
end
|
||||||
|
|
||||||
def current_runner
|
def current_runner
|
||||||
|
|
|
@ -526,6 +526,15 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
|
||||||
expect(runner.reload.ip_address).to eq('123.222.123.222')
|
expect(runner.reload.ip_address).to eq('123.222.123.222')
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "handles multiple X-Forwarded-For addresses" do
|
||||||
|
post api('/jobs/request'),
|
||||||
|
params: { token: runner.token },
|
||||||
|
headers: { 'User-Agent' => user_agent, 'X-Forwarded-For' => '123.222.123.222, 127.0.0.1' }
|
||||||
|
|
||||||
|
expect(response).to have_gitlab_http_status 201
|
||||||
|
expect(runner.reload.ip_address).to eq('123.222.123.222')
|
||||||
|
end
|
||||||
|
|
||||||
context 'when concurrently updating a job' do
|
context 'when concurrently updating a job' do
|
||||||
before do
|
before do
|
||||||
expect_any_instance_of(Ci::Build).to receive(:run!)
|
expect_any_instance_of(Ci::Build).to receive(:run!)
|
||||||
|
|
Loading…
Reference in a new issue