summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2017-11-08 16:21:41 +0000
committerRémy Coutable <remy@rymai.me>2017-11-08 16:21:41 +0000
commit07ab4ad60a894d60ac561f9fdbd641fac90b4acc (patch)
tree9817869e4ed30b9ab05480c8b3daa6776148e557
parent9e294180f29ec7ec9e9734a65b2b5d3124d657e4 (diff)
parent0232450c8aee08a656275caf7b990e0bcbbb1cf4 (diff)
downloadgitlab-ce-07ab4ad60a894d60ac561f9fdbd641fac90b4acc.tar.gz
Merge branch 'sh-fix-lfs-write-deploy-keys' into 'master'
Fix Error 500 when pushing LFS objects with a write deploy key Closes #39467 See merge request gitlab-org/gitlab-ce!15039
-rw-r--r--app/controllers/concerns/lfs_request.rb3
-rw-r--r--lib/gitlab/auth.rb15
-rw-r--r--lib/gitlab/lfs_token.rb4
-rw-r--r--spec/lib/gitlab/auth_spec.rb28
-rw-r--r--spec/requests/lfs_http_spec.rb47
5 files changed, 69 insertions, 28 deletions
diff --git a/app/controllers/concerns/lfs_request.rb b/app/controllers/concerns/lfs_request.rb
index 738afd612f0..6cca9f95618 100644
--- a/app/controllers/concerns/lfs_request.rb
+++ b/app/controllers/concerns/lfs_request.rb
@@ -74,8 +74,9 @@ module LfsRequest
def lfs_upload_access?
return false unless project.lfs_enabled?
+ return false unless has_authentication_ability?(:push_code)
- has_authentication_ability?(:push_code) && can?(user, :push_code, project)
+ lfs_deploy_token? || can?(user, :push_code, project)
end
def lfs_deploy_token?
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 0ad9285c0ea..cbbc51db99e 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -25,7 +25,7 @@ module Gitlab
result =
service_request_check(login, password, project) ||
build_access_token_check(login, password) ||
- lfs_token_check(login, password) ||
+ lfs_token_check(login, password, project) ||
oauth_access_token_check(login, password) ||
personal_access_token_check(password) ||
user_with_password_for_git(login, password) ||
@@ -146,7 +146,7 @@ module Gitlab
end.flatten.uniq
end
- def lfs_token_check(login, password)
+ def lfs_token_check(login, password, project)
deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/)
actor =
@@ -163,6 +163,8 @@ module Gitlab
authentication_abilities =
if token_handler.user?
full_authentication_abilities
+ elsif token_handler.deploy_key_pushable?(project)
+ read_write_authentication_abilities
else
read_authentication_abilities
end
@@ -208,10 +210,15 @@ module Gitlab
]
end
- def full_authentication_abilities
+ def read_write_authentication_abilities
read_authentication_abilities + [
:push_code,
- :create_container_image,
+ :create_container_image
+ ]
+ end
+
+ def full_authentication_abilities
+ read_write_authentication_abilities + [
:admin_container_image
]
end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index 8e57ba831c5..ead5d566871 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -27,6 +27,10 @@ module Gitlab
end
end
+ def deploy_key_pushable?(project)
+ actor.is_a?(DeployKey) && actor.can_push_to?(project)
+ end
+
def user?
actor.is_a?(User)
end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 54a853c9ce3..3164d2ebf04 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -133,6 +133,25 @@ describe Gitlab::Auth do
gl_auth.find_for_git_client(user.username, token, project: nil, ip: 'ip')
end
+
+ it 'grants deploy key write permissions' do
+ project = create(:project)
+ key = create(:deploy_key, can_push: true)
+ create(:deploy_keys_project, deploy_key: key, project: project)
+ token = Gitlab::LfsToken.new(key).token
+
+ expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}")
+ expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: project, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_write_authentication_abilities))
+ end
+
+ it 'does not grant deploy key write permissions' do
+ project = create(:project)
+ key = create(:deploy_key, can_push: true)
+ token = Gitlab::LfsToken.new(key).token
+
+ expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}")
+ expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: project, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities))
+ end
end
context 'while using OAuth tokens as passwords' do
@@ -326,10 +345,15 @@ describe Gitlab::Auth do
]
end
- def full_authentication_abilities
+ def read_write_authentication_abilities
read_authentication_abilities + [
:push_code,
- :create_container_image,
+ :create_container_image
+ ]
+ end
+
+ def full_authentication_abilities
+ read_write_authentication_abilities + [
:admin_container_image
]
end
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 52e93e157f1..c597623bc4d 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -654,6 +654,20 @@ describe 'Git LFS API and storage' do
}
end
+ shared_examples 'pushes new LFS objects' do
+ let(:sample_size) { 150.megabytes }
+ let(:sample_oid) { '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897' }
+
+ it 'responds with upload hypermedia link' do
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['objects']).to be_kind_of(Array)
+ expect(json_response['objects'].first['oid']).to eq(sample_oid)
+ expect(json_response['objects'].first['size']).to eq(sample_size)
+ expect(json_response['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.full_path}.git/gitlab-lfs/objects/#{sample_oid}/#{sample_size}")
+ expect(json_response['objects'].first['actions']['upload']['header']).to eq('Authorization' => authorization)
+ end
+ end
+
describe 'when request is authenticated' do
describe 'when user has project push access' do
let(:authorization) { authorize_user }
@@ -684,27 +698,7 @@ describe 'Git LFS API and storage' do
end
context 'when pushing a lfs object that does not exist' do
- let(:body) do
- {
- 'operation' => 'upload',
- 'objects' => [
- { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
- 'size' => 1575078 }
- ]
- }
- end
-
- it 'responds with status 200' do
- expect(response).to have_gitlab_http_status(200)
- end
-
- it 'responds with upload hypermedia link' do
- expect(json_response['objects']).to be_kind_of(Array)
- expect(json_response['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897")
- expect(json_response['objects'].first['size']).to eq(1575078)
- expect(json_response['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.full_path}.git/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/1575078")
- expect(json_response['objects'].first['actions']['upload']['header']).to eq('Authorization' => authorization)
- end
+ it_behaves_like 'pushes new LFS objects'
end
context 'when pushing one new and one existing lfs object' do
@@ -785,6 +779,17 @@ describe 'Git LFS API and storage' do
end
end
end
+
+ context 'when deploy key has project push access' do
+ let(:key) { create(:deploy_key, can_push: true) }
+ let(:authorization) { authorize_deploy_key }
+
+ let(:update_user_permissions) do
+ project.deploy_keys << key
+ end
+
+ it_behaves_like 'pushes new LFS objects'
+ end
end
context 'when user is not authenticated' do