diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb
index 6b4382352584d6a78c774cdbc339dea99f428d70..9a80671f996f6aedd825d2e8618ea12114e0640e 100644
--- a/lib/api/helpers/internal_helpers.rb
+++ b/lib/api/helpers/internal_helpers.rb
@@ -17,6 +17,18 @@ def project
@project # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
+ def access_checker_for(actor, protocol)
+ access_checker_klass.new(actor.key_or_user, project, protocol,
+ authentication_abilities: ssh_authentication_abilities,
+ namespace_path: namespace_path,
+ project_path: project_path,
+ redirected_path: redirected_path)
+ end
+
+ def access_checker_klass
+ repo_type.access_checker_class
+ end
+
def ssh_authentication_abilities
[
:read_project,
diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb
index 622032b8355606293525b47fd00e7066cbb9ada8..087985d34b0a088ff501eb182ccbc36b519f54bf 100644
--- a/lib/api/internal/base.rb
+++ b/lib/api/internal/base.rb
@@ -35,36 +35,14 @@ def lfs_authentication_url(project)
# 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
- # rubocop: disable CodeReuse/ActiveRecord
post "/allowed" do
# Stores some Git-specific env thread-safely
env = parse_env
Gitlab::Git::HookEnv.set(gl_repository, env) if project
- actor =
- if params[:key_id]
- Key.find_by(id: params[:key_id])
- elsif params[:user_id]
- User.find_by(id: params[:user_id])
- elsif params[:username]
- UserFinder.new(params[:username]).find_by_username
- end
-
- protocol = params[:protocol]
-
- actor.update_last_used_at if actor.is_a?(Key)
- user =
- if actor.is_a?(Key)
- actor.user
- else
- actor
- end
-
- access_checker_klass = repo_type.access_checker_class
- access_checker = access_checker_klass.new(actor, project,
- protocol, authentication_abilities: ssh_authentication_abilities,
- namespace_path: namespace_path, project_path: project_path,
- redirected_path: redirected_path)
+ actor = Support::GitAccessActor.from_params(params)
+ actor.update_last_used_at!
+ access_checker = access_checker_for(actor, params[:protocol])
check_result = begin
result = access_checker.check(params[:action], params[:changes])
@@ -78,15 +56,15 @@ def lfs_authentication_url(project)
break response_with_status(code: 404, success: false, message: e.message)
end
- log_user_activity(actor)
+ log_user_activity(actor.user)
case check_result
when ::Gitlab::GitAccessResult::Success
payload = {
gl_repository: gl_repository,
gl_project_path: gl_project_path,
- gl_id: Gitlab::GlId.gl_id(user),
- gl_username: user&.username,
+ gl_id: Gitlab::GlId.gl_id(actor.user),
+ gl_username: actor.username,
git_config_options: [],
gitaly: gitaly_payload(params[:action]),
gl_console_messages: check_result.console_messages
@@ -105,7 +83,6 @@ def lfs_authentication_url(project)
response_with_status(code: 500, success: false, message: UNKNOWN_CHECK_RESULT_ERROR)
end
end
- # rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
post "/lfs_authenticate" do
diff --git a/lib/api/support/git_access_actor.rb b/lib/api/support/git_access_actor.rb
new file mode 100644
index 0000000000000000000000000000000000000000..2e0962c6295526e155d3f160667a5a7dceb8413f
--- /dev/null
+++ b/lib/api/support/git_access_actor.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module API
+ module Support
+ class GitAccessActor
+ attr_reader :user
+
+ def initialize(user: nil, key: nil)
+ @user = user
+ @key = key
+
+ @user = key.user if !user && key
+ end
+
+ def self.from_params(params)
+ if params[:key_id]
+ new(key: Key.find_by_id(params[:key_id]))
+ elsif params[:user_id]
+ new(user: UserFinder.new(params[:user_id]).find_by_id)
+ elsif params[:username]
+ new(user: UserFinder.new(params[:username]).find_by_username)
+ end
+ end
+
+ def key_or_user
+ key || user
+ end
+
+ def username
+ user&.username
+ end
+
+ def update_last_used_at!
+ key&.update_last_used_at
+ end
+
+ private
+
+ attr_reader :key
+ end
+ end
+end
diff --git a/spec/lib/api/support/git_access_actor_spec.rb b/spec/lib/api/support/git_access_actor_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..63f5966faea46646da4ac6436469a13b2af7172a
--- /dev/null
+++ b/spec/lib/api/support/git_access_actor_spec.rb
@@ -0,0 +1,128 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::Support::GitAccessActor do
+ let(:user) { nil }
+ let(:key) { nil }
+
+ subject { described_class.new(user: user, key: key) }
+
+ describe '.from_params' do
+ context 'with params that are valid' do
+ it 'returns an instance of API::Support::GitAccessActor' do
+ params = { key_id: create(:key).id }
+
+ expect(described_class.from_params(params)).to be_instance_of(described_class)
+ end
+ end
+
+ context 'with params that are invalid' do
+ it 'returns nil' do
+ expect(described_class.from_params({})).to be_nil
+ end
+ end
+ end
+
+ describe 'attributes' do
+ describe '#user' do
+ context 'when initialized with a User' do
+ let(:user) { create(:user) }
+
+ it 'returns the User' do
+ expect(subject.user).to eq(user)
+ end
+ end
+
+ context 'when initialized with a Key' do
+ let(:user_for_key) { create(:user) }
+ let(:key) { create(:key, user: user_for_key) }
+
+ it 'returns the User associated to the Key' do
+ expect(subject.user).to eq(user_for_key)
+ end
+ end
+ end
+ end
+
+ describe '#key_or_user' do
+ context 'when params contains a :key_id' do
+ it 'is an instance of Key' do
+ key = create(:key)
+ params = { key_id: key.id }
+
+ expect(described_class.from_params(params).key_or_user).to eq(key)
+ end
+ end
+
+ context 'when params contains a :user_id' do
+ it 'is an instance of User' do
+ user = create(:user)
+ params = { user_id: user.id }
+
+ expect(described_class.from_params(params).key_or_user).to eq(user)
+ end
+ end
+
+ context 'when params contains a :username' do
+ it 'is an instance of User (via UserFinder)' do
+ user = create(:user)
+ params = { username: user.username }
+
+ expect(described_class.from_params(params).key_or_user).to eq(user)
+ end
+ end
+ end
+
+ describe '#username' do
+ context 'when initialized with a User' do
+ let(:user) { create(:user) }
+
+ it 'returns the username' do
+ expect(subject.username).to eq(user.username)
+ end
+ end
+
+ context 'when initialized with a Key' do
+ let(:key) { create(:key, user: user_for_key) }
+
+ context 'that has no User associated' do
+ let(:user_for_key) { nil }
+
+ it 'returns nil' do
+ expect(subject.username).to be_nil
+ end
+ end
+
+ context 'that has a User associated' do
+ let(:user_for_key) { create(:user) }
+
+ it 'returns the username of the User associated to the Key' do
+ expect(subject.username).to eq(user_for_key.username)
+ end
+ end
+ end
+ end
+
+ describe '#update_last_used_at!' do
+ context 'when initialized with a User' do
+ let(:user) { create(:user) }
+
+ it 'does nothing' do
+ expect(user).not_to receive(:update_last_used_at)
+
+ subject.update_last_used_at!
+ end
+ end
+
+ context 'when initialized with a Key' do
+ let(:key) { create(:key) }
+
+ it 'updates update_last_used_at' do
+ expect(key).to receive(:update_last_used_at)
+
+ subject.update_last_used_at!
+ end
+ end
+ end
+end