Commit 9b182cbd authored by Gosia Ksionek's avatar Gosia Ksionek Committed by Michael Kozono

Refactor IP address state class

Add middleware to api

Add new middleware class

Cast ip to string

Add specyfication for groups endpoint

Add specs for projects endpoint

Add working specs for ip_saver

Add changelog entry

Add string freeze

Fix rubocop offences

Add code review remarks

Fix typo in filename

Add docs about api restriction

Update docs file

Add changelod

Remove not needed file

Update docs file

Add changelod

Remove changelog

Apply suggestion to doc/user/group/index.md

Add code review remarks

Remove not needed code

Bring back to_s method on remote ip

Add if clause to restrictor

To avoid locking out of ssh calls

Make regex wider so it covers all internal endpoints

Add remote ip to be used in middleware

Add rubocop remarks
parent 290f2672
...@@ -346,7 +346,7 @@ Add one or more whitelisted IP subnets using CIDR notation in comma separated fo ...@@ -346,7 +346,7 @@ Add one or more whitelisted IP subnets using CIDR notation in comma separated fo
coming from a different IP address won't be able to access the restricted coming from a different IP address won't be able to access the restricted
content. content.
Restriction currently applies to UI, API access is not restricted. Restriction currently applies to UI and API access, Git actions via ssh are not restricted.
To avoid accidental lock-out, admins and group owners are are able to access To avoid accidental lock-out, admins and group owners are are able to access
the group regardless of the IP restriction. the group regardless of the IP restriction.
......
---
title: Apply the group setting "Restrict access by IP address" to API requests
merge_request: 15282
author:
type: changed
...@@ -2,10 +2,12 @@ ...@@ -2,10 +2,12 @@
module EE module EE
module API module API
module Endpoints module API
extend ActiveSupport::Concern extend ActiveSupport::Concern
prepended do prepended do
use ::Gitlab::Middleware::IpRestrictor
mount ::EE::API::Boards mount ::EE::API::Boards
mount ::EE::API::GroupBoards mount ::EE::API::GroupBoards
......
...@@ -6,9 +6,17 @@ module Gitlab ...@@ -6,9 +6,17 @@ module Gitlab
class << self class << self
def with(address) def with(address)
self.current = address set_address(address)
yield yield
ensure ensure
nullify_address
end
def set_address(address)
self.current = address
end
def nullify_address
self.current = nil self.current = nil
end end
......
# frozen_string_literal: true
module Gitlab
module Middleware
class IpRestrictor
def initialize(app)
@app = app
end
def call(env)
return @app.call(env) if env['PATH_INFO'] =~ %r{^/api/v\d+/internal/}
::Gitlab::IpAddressState.with(env['action_dispatch.remote_ip'].to_s) do # rubocop: disable CodeReuse/ActiveRecord
@app.call(env)
end
end
end
end
end
...@@ -3,9 +3,8 @@ ...@@ -3,9 +3,8 @@
require 'spec_helper' require 'spec_helper'
describe Gitlab::IpAddressState do describe Gitlab::IpAddressState do
let(:address) { '1.1.1.1' }
describe '.with' do describe '.with' do
let(:address) { '1.1.1.1' }
it 'saves IP address' do it 'saves IP address' do
described_class.with(address) do described_class.with(address) do
expect(Thread.current[described_class::THREAD_KEY]).to eq(address) expect(Thread.current[described_class::THREAD_KEY]).to eq(address)
...@@ -26,4 +25,20 @@ describe Gitlab::IpAddressState do ...@@ -26,4 +25,20 @@ describe Gitlab::IpAddressState do
expect(Thread.current[described_class::THREAD_KEY]).to eq(nil) expect(Thread.current[described_class::THREAD_KEY]).to eq(nil)
end end
end end
describe '.set_address' do
it 'saves IP address' do
described_class.set_address(address) do
expect(Thread.current[described_class::THREAD_KEY]).to eq(address)
end
end
end
describe '.nullify_address' do
it 'clears IP address' do
described_class.nullify_address do
expect(Thread.current[described_class::THREAD_KEY]).to eq(nil)
end
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Middleware::IpRestrictor do
let(:app) { double(:app) }
let(:middleware) { described_class.new(app) }
let(:env) { {} }
describe '#call' do
before do
allow(env).to receive(:[]).with('action_dispatch.remote_ip').and_return('127.0.0.1')
allow(env).to receive(:[]).with('PATH_INFO').and_return('/api/v4/groups')
end
it 'calls ip address state to set the address' do
expect(::Gitlab::IpAddressState).to receive(:set_address).with('127.0.0.1')
expect(app).to receive(:call)
middleware.call(env)
end
it 'calls ip address state to nullify the address' do
expect(::Gitlab::IpAddressState).to receive(:nullify_address)
expect(app).to receive(:call)
middleware.call(env)
end
it 'calls ip address state to nullify the address when app raises an error' do
expect(::Gitlab::IpAddressState).to receive(:nullify_address)
expect(app).to receive(:call).and_raise('boom')
expect { middleware.call(env) }.to raise_error
end
context 'when it is internal endpoint' do
before do
allow(env).to receive(:[]).with('PATH_INFO').and_return('/api/v4/internal/allowed')
end
it 'calls ip address state to set the address' do
expect(::Gitlab::IpAddressState).not_to receive(:with)
expect(app).to receive(:call)
middleware.call(env)
end
end
end
end
...@@ -39,6 +39,43 @@ describe API::Groups do ...@@ -39,6 +39,43 @@ describe API::Groups do
end end
end end
describe 'GET /groups/:id' do
before do
create(:ip_restriction, group: private_group)
private_group.add_maintainer(user)
end
context 'when the group_ip_restriction feature is not available' do
before do
stub_licensed_features(group_ip_restriction: false)
end
it 'returns 200' do
get api("/groups/#{private_group.id}", user)
expect(response).to have_gitlab_http_status(200)
end
end
context 'when the group_ip_restriction feature is available' do
before do
stub_licensed_features(group_ip_restriction: true)
end
it 'returns 404 for request from ip not in the range' do
get api("/groups/#{private_group.id}", user)
expect(response).to have_gitlab_http_status(404)
end
it 'returns 200 for request from ip in the range' do
get api("/groups/#{private_group.id}", user), headers: { 'REMOTE_ADDR' => '192.168.0.0' }
expect(response).to have_gitlab_http_status(200)
end
end
end
describe 'PUT /groups/:id' do describe 'PUT /groups/:id' do
subject { put api("/groups/#{group.id}", user), params: params } subject { put api("/groups/#{group.id}", user), params: params }
......
...@@ -100,6 +100,46 @@ describe API::Projects do ...@@ -100,6 +100,46 @@ describe API::Projects do
expect(json_response['external_authorization_classification_label']).to be_nil expect(json_response['external_authorization_classification_label']).to be_nil
end end
end end
context 'with ip restriction' do
let(:group) { create :group, :private }
before do
create(:ip_restriction, group: group)
group.add_maintainer(user)
project.update(namespace: group)
end
context 'when the group_ip_restriction feature is not available' do
before do
stub_licensed_features(group_ip_restriction: false)
end
it 'returns 200' do
get api("/projects/#{project.id}", user)
expect(response).to have_gitlab_http_status(200)
end
end
context 'when the group_ip_restriction feature is available' do
before do
stub_licensed_features(group_ip_restriction: true)
end
it 'returns 404 for request from ip not in the range' do
get api("/projects/#{project.id}", user)
expect(response).to have_gitlab_http_status(404)
end
it 'returns 200 for request from ip in the range' do
get api("/projects/#{project.id}", user), headers: { 'REMOTE_ADDR' => '192.168.0.0' }
expect(response).to have_gitlab_http_status(200)
end
end
end
end end
describe 'packages_enabled attribute' do describe 'packages_enabled attribute' do
......
...@@ -181,4 +181,4 @@ module API ...@@ -181,4 +181,4 @@ module API
end end
end end
API::API.prepend_if_ee('::EE::API::Endpoints') API::API.prepend_if_ee('::EE::API::API')
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