Merge branch '12874-extend-group-ip-restriction-to-command-line' into 'master'

Apply the group setting "Restrict access by IP address" to API requests

See merge request gitlab-org/gitlab-ee!15282

doc/user/group/index.md

@@ -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
 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
 the group regardless of the IP restriction.
 

ee/changelogs/unreleased/12874-extend-group-ip-restriction-to-command-line.yml

+---
+title: Apply the group setting "Restrict access by IP address" to API requests
+merge_request: 15282
+author:
+type: changed

ee/lib/ee/api/endpoints.rb → ee/lib/ee/api/api.rb

@@ -2,10 +2,12 @@
 
 module EE
   module API
-    module Endpoints
+    module API
       extend ActiveSupport::Concern
 
       prepended do
+        use ::Gitlab::Middleware::IpRestrictor
+
         mount ::EE::API::Boards
         mount ::EE::API::GroupBoards
 

ee/lib/gitlab/ip_address_state.rb

@@ -6,9 +6,17 @@ module Gitlab
 
     class << self
       def with(address)
-        self.current = address
+        set_address(address)
         yield
       ensure
+        nullify_address
+      end
+
+      def set_address(address)
+        self.current = address
+      end
+
+      def nullify_address
         self.current = nil
       end
 

ee/lib/gitlab/middleware/ip_restrictor.rb

+# 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

ee/spec/lib/gitlab/ip_address_state_spec.rb

@@ -3,9 +3,8 @@
 require 'spec_helper'
 
 describe Gitlab::IpAddressState do
+  let(:address) { '1.1.1.1' }
   describe '.with' do
-    let(:address) { '1.1.1.1' }
-
     it 'saves IP address' do
       described_class.with(address) do
         expect(Thread.current[described_class::THREAD_KEY]).to eq(address)
@@ -26,4 +25,20 @@ describe Gitlab::IpAddressState do
       expect(Thread.current[described_class::THREAD_KEY]).to eq(nil)
     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

ee/spec/lib/gitlab/middleware/ip_restrictor_spec.rb

+# 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

ee/spec/requests/api/groups_spec.rb

@@ -39,6 +39,43 @@ describe API::Groups do
     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
     subject { put api("/groups/#{group.id}", user), params: params }
 

ee/spec/requests/api/projects_spec.rb

@@ -100,6 +100,46 @@ describe API::Projects do
           expect(json_response['external_authorization_classification_label']).to be_nil
         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
 
     describe 'packages_enabled attribute' do

lib/api/api.rb

@@ -181,4 +181,4 @@ module API
   end
 end
 
-API::API.prepend_if_ee('::EE::API::Endpoints')
+API::API.prepend_if_ee('::EE::API::API')