diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index a13e7b9c87e49f1dfb6e21b7aba96d80d1b6ed82..4149c39eec6fafa82d256ecc53771684108f2242 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -10.0.0 +10.1.0 diff --git a/doc/development/internal_api.md b/doc/development/internal_api.md index 2b8a20939ef0045f704656f928df380de5ae37b7..b08112aacb23fb21a773eb338f08ade3bc1457e9 100644 --- a/doc/development/internal_api.md +++ b/doc/development/internal_api.md @@ -47,6 +47,7 @@ POST /internal/allowed | `protocol` | string | yes | SSH when called from GitLab-shell, HTTP or SSH when called from Gitaly | | `action` | string | yes | Git command being run (`git-upload-pack`, `git-receive-pack`, `git-upload-archive`) | | `changes` | string | yes | `<oldrev> <newrev> <refname>` when called from Gitaly, The magic string `_any` when called from GitLab Shell | +| `check_ip` | string | no | Ip adress from which call to GitLab Shell was made | Example request: diff --git a/doc/user/group/index.md b/doc/user/group/index.md index 0dbf5bdd15663ce4d7a856c799f3d10024499f46..258f1264b489123bbd7d4c65ee35e050937d475d 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -351,7 +351,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 and API access, Git actions via SSH are not restricted. +Restriction currently applies to UI, API access and Git actions via SSH. To avoid accidental lock-out, admins and group owners are are able to access the group regardless of the IP restriction. diff --git a/ee/changelogs/unreleased/32113-extend-group-ip-restriction-to-git-activity.yml b/ee/changelogs/unreleased/32113-extend-group-ip-restriction-to-git-activity.yml new file mode 100644 index 0000000000000000000000000000000000000000..545d5265fc9460801e2a943a70cc3ab6741b2391 --- /dev/null +++ b/ee/changelogs/unreleased/32113-extend-group-ip-restriction-to-git-activity.yml @@ -0,0 +1,5 @@ +--- +title: Extend group IP restriction to Git activity +merge_request: 16980 +author: +type: added diff --git a/ee/lib/ee/api/internal/base.rb b/ee/lib/ee/api/internal/base.rb index ebc7cba92ff1839b7fab1d41dc8a407401a056bd..6e73c8d15cc2aad837270dc86c2f07e191291018 100644 --- a/ee/lib/ee/api/internal/base.rb +++ b/ee/lib/ee/api/internal/base.rb @@ -34,6 +34,14 @@ def current_replication_lag def fetch_geo_node_referrer ::Gitlab::Geo::GitPushHttp.new(params[:identifier], params[:gl_repository]).fetch_referrer_node end + + override :check_allowed + def check_allowed(params) + ip = params.fetch(:check_ip, nil) + ::Gitlab::IpAddressState.with(ip) do # rubocop: disable CodeReuse/ActiveRecord + super + end + end end end end diff --git a/ee/spec/lib/gitlab/middleware/ip_restrictor_spec.rb b/ee/spec/lib/gitlab/middleware/ip_restrictor_spec.rb index be50b329cc78318a8c5f685d2a93dd29831c29b9..cd04b65469cfcba2c1410df9f6ee3e007235dc01 100644 --- a/ee/spec/lib/gitlab/middleware/ip_restrictor_spec.rb +++ b/ee/spec/lib/gitlab/middleware/ip_restrictor_spec.rb @@ -38,7 +38,7 @@ allow(env).to receive(:[]).with('PATH_INFO').and_return('/api/v4/internal/allowed') end - it 'calls ip address state to set the address' do + it 'does not call ip address state to set the address' do expect(::Gitlab::IpAddressState).not_to receive(:with) expect(app).to receive(:call) diff --git a/ee/spec/requests/api/internal/base_spec.rb b/ee/spec/requests/api/internal/base_spec.rb index a6aca29a33884824209d4f2a7b6cb20f7002efc0..a1e09b18d115a2b55f9f4edce984bde15b8ba64a 100644 --- a/ee/spec/requests/api/internal/base_spec.rb +++ b/ee/spec/requests/api/internal/base_spec.rb @@ -252,6 +252,54 @@ def check_access_by_alias(alias_name) end end end + + context 'ip restriction' do + let_it_be(:group) { create(:group)} + let_it_be(:project) { create(:project, :repository, namespace: group) } + let(:params) do + { + key_id: key.id, + project: project.full_path, + gl_repository: "project-#{project.id}", + action: 'git-upload-pack', + secret_token: secret_token, + protocol: 'ssh' + } + end + let(:allowed_ip) { '150.168.0.1' } + + before do + create(:ip_restriction, group: group, range: allowed_ip) + stub_licensed_features(group_ip_restriction: true) + + project.add_developer(user) + end + + context 'with or without check_ip parameter' do + using RSpec::Parameterized::TableSyntax + + where(:check_ip_present, :ip, :status) do + false | nil | 200 + true | '150.168.0.1' | 200 + true | '150.168.0.2' | 404 + end + + with_them do + subject do + post( + api('/internal/allowed'), + params: check_ip_present ? params.merge(check_ip: ip) : params + ) + end + + it 'modifies access' do + subject + + expect(response).to have_gitlab_http_status(status) + end + end + end + end end describe "POST /internal/lfs_authenticate", :geo do diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index 00e9b56b932da4dec14d45efcfa59dbeb3cd5dc5..d9a22484c1ff3ff5135588396a87e2d8b4511eaf 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -26,20 +26,11 @@ def lfs_authentication_url(project) def ee_post_receive_response_hook(response) # Hook for EE to add messages end - end - namespace 'internal' do - # Check if git command is allowed for project - # - # Params: - # key_id - ssh key id for Git over SSH - # user_id - user id for Git over HTTP or over SSH in keyless SSH CERT mode - # username - user name for Git over SSH in keyless SSH cert mode - # protocol - Git access protocol being used, e.g. HTTP or SSH - # project - project full_path (not path on disk) - # action - git action (git-upload-pack or git-receive-pack) - # changes - changes as "oldrev newrev ref", see Gitlab::ChangesList - post "/allowed" do + def check_allowed(params) + # This is a separate method so that EE can alter its behaviour more + # easily. + # Stores some Git-specific env thread-safely env = parse_env Gitlab::Git::HookEnv.set(gl_repository, env) if project @@ -53,11 +44,11 @@ def ee_post_receive_response_hook(response) @project ||= access_checker.project result rescue Gitlab::GitAccess::UnauthorizedError => e - break response_with_status(code: 401, success: false, message: e.message) + return response_with_status(code: 401, success: false, message: e.message) rescue Gitlab::GitAccess::TimeoutError => e - break response_with_status(code: 503, success: false, message: e.message) + return response_with_status(code: 503, success: false, message: e.message) rescue Gitlab::GitAccess::NotFoundError => e - break response_with_status(code: 404, success: false, message: e.message) + return response_with_status(code: 404, success: false, message: e.message) end log_user_activity(actor.user) @@ -91,6 +82,26 @@ def ee_post_receive_response_hook(response) response_with_status(code: 500, success: false, message: UNKNOWN_CHECK_RESULT_ERROR) end end + end + + namespace 'internal' do + # Check if git command is allowed for project + # + # Params: + # key_id - ssh key id for Git over SSH + # user_id - user id for Git over HTTP or over SSH in keyless SSH CERT mode + # username - user name for Git over SSH in keyless SSH cert mode + # protocol - Git access protocol being used, e.g. HTTP or SSH + # project - project full_path (not path on disk) + # action - git action (git-upload-pack or git-receive-pack) + # changes - changes as "oldrev newrev ref", see Gitlab::ChangesList + # check_ip - optional, only in EE version, may limit access to + # group resources based on its IP restrictions + post "/allowed" do + # It was moved to a separate method so that EE can alter its behaviour more + # easily. + check_allowed(params) + end # rubocop: disable CodeReuse/ActiveRecord post "/lfs_authenticate" do