Commit 0232450c authored by Stan Hu's avatar Stan Hu Committed by Rémy Coutable

Fix Error 500 when pushing LFS objects with a write deploy key

parent 9e294180
......@@ -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?
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
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